Fixed bug #68583 Crash in timeout thread

This replaces the GUI element used for execution timeout handling
on Windows. Instead a timer queue technique is used, which is indeed
a thread pool. A timer queue timer is a lightweight object handled
but that thread pool and the timer thread spends most of the time
sleeping and waiting for an alert.

Please note also that this introduces neither binary nor source
breach. The custom timeout thread functions are deleted, however
they was not exported throug DLL, so couldn't be used by any
external code. As well they couldn't be used anywhere in the core
except in executor api, because those custom timeout thread
functions they used to operate on static variables which would
be overwritten (and that would blow).

So instead a relatively modern technique is used for the timeout
handling. It's still not perfect because the executor still has to
check EX(timed_out). This can be a topic for an improvement in
master. But brobably can be tricky as currently it seems to be not
possible to signal an individual thread. Also note another issue
that static variables aren't thread safe, but the current timer
implementation is.

1 files changed, 41 insertions, 118 deletions

diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index f5ee01463a..d6143807ca 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -46,13 +46,10 @@ ZEND_API const zend_fcall_info empty_fcall_info = { 0, NULL, NULL, NULL, NULL, 0
 ZEND_API const zend_fcall_info_cache empty_fcall_info_cache = { 0, NULL, NULL, NULL, NULL };
 
 #ifdef ZEND_WIN32
-#include <process.h>
-static WNDCLASS wc;
-static HWND timeout_window;
-static HANDLE timeout_thread_event;
-static HANDLE timeout_thread_handle;
-static DWORD timeout_thread_id;
-static int timeout_thread_initialized=0;
+#ifdef ZTS
+__declspec(thread)
+#endif
+HANDLE tq_timer = NULL;
 #endif
 
 #if 0&&ZEND_DEBUG
@@ -1339,115 +1336,19 @@ ZEND_API void zend_timeout(int dummy) /* {{{ */
 /* }}} */
 
 #ifdef ZEND_WIN32
-static LRESULT CALLBACK zend_timeout_WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) /* {{{ */
-{
-#ifdef ZTS
-	THREAD_T thread_id = (THREAD_T)wParam;
-#endif
-
-	switch (message) {
-		case WM_DESTROY:
-			PostQuitMessage(0);
-			break;
-		case WM_REGISTER_ZEND_TIMEOUT:
-			/* wParam is the thread id pointer, lParam is the timeout amount in seconds */
-			if (lParam == 0) {
-				KillTimer(timeout_window, wParam);
-			} else {
-#ifdef ZTS
-				void ***tsrm_ls;
-#endif
-				SetTimer(timeout_window, wParam, lParam*1000, NULL);
-#ifdef ZTS
-				tsrm_ls = ts_resource_ex(0, &thread_id);
-				if (!tsrm_ls) {
-					/* shouldn't normally happen */
-					break;
-				}
-#endif
-				EG(timed_out) = 0;
-			}
-			break;
-		case WM_UNREGISTER_ZEND_TIMEOUT:
-			/* wParam is the thread id pointer */
-			KillTimer(timeout_window, wParam);
-			break;
-		case WM_TIMER: {
-#ifdef ZTS
-				void ***tsrm_ls;
-
-				tsrm_ls = ts_resource_ex(0, &thread_id);
-				if (!tsrm_ls) {
-					/* Thread died before receiving its timeout? */
-					break;
-				}
-#endif
-				KillTimer(timeout_window, wParam);
-				EG(timed_out) = 1;
-			}
-			break;
-		default:
-			return DefWindowProc(hWnd, message, wParam, lParam);
-	}
-	return 0;
-}
-/* }}} */
-
-static unsigned __stdcall timeout_thread_proc(void *pArgs) /* {{{ */
-{
-	MSG message;
-
-	wc.style=0;
-	wc.lpfnWndProc = zend_timeout_WndProc;
-	wc.cbClsExtra=0;
-	wc.cbWndExtra=0;
-	wc.hInstance=NULL;
-	wc.hIcon=NULL;
-	wc.hCursor=NULL;
-	wc.hbrBackground=(HBRUSH)(COLOR_BACKGROUND + 5);
-	wc.lpszMenuName=NULL;
-	wc.lpszClassName = "Zend Timeout Window";
-	if (!RegisterClass(&wc)) {
-		return -1;
-	}
-	timeout_window = CreateWindow(wc.lpszClassName, wc.lpszClassName, 0, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, NULL, NULL);
-	SetEvent(timeout_thread_event);
-	while (GetMessage(&message, NULL, 0, 0)) {
-		SendMessage(timeout_window, message.message, message.wParam, message.lParam);
-		if (message.message == WM_QUIT) {
-			break;
-		}
-	}
-	DestroyWindow(timeout_window);
-	UnregisterClass(wc.lpszClassName, NULL);
-	SetEvent(timeout_thread_handle);
-	return 0;
-}
-/* }}} */
-
-void zend_init_timeout_thread(void) /* {{{ */
+VOID CALLBACK tq_timer_cb(PVOID arg, BOOLEAN timed_out)
 {
-	timeout_thread_event = CreateEvent(NULL, FALSE, FALSE, NULL);
-	timeout_thread_handle = CreateEvent(NULL, FALSE, FALSE, NULL);
-	_beginthreadex(NULL, 0, timeout_thread_proc, NULL, 0, &timeout_thread_id);
-	WaitForSingleObject(timeout_thread_event, INFINITE);
-}
-/* }}} */
+	zend_bool *php_timed_out;
 
-void zend_shutdown_timeout_thread(void) /* {{{ */
-{
-	if (!timeout_thread_initialized) {
+	/* The doc states it'll be always true, however it theoretically
+		could be FALSE when the thread was signaled. */
+	if (!timed_out) {
 		return;
 	}
-	PostThreadMessage(timeout_thread_id, WM_QUIT, 0, 0);
 
-	/* Wait for thread termination */
-	WaitForSingleObject(timeout_thread_handle, 5000);
-	CloseHandle(timeout_thread_handle);
-	timeout_thread_initialized = 0;
+	php_timed_out = (zend_bool *)arg;
+	*php_timed_out = 1;
 }
-/* }}} */
-
 #endif
 
 /* This one doesn't exists on QNX */
@@ -1465,13 +1366,28 @@ void zend_set_timeout(long seconds, int reset_signals) /* {{{ */
 	if(!seconds) {
 		return;
 	}
-	if (timeout_thread_initialized == 0 && InterlockedIncrement(&timeout_thread_initialized) == 1) {
-		/* We start up this process-wide thread here and not in zend_startup(), because if Zend
-		 * is initialized inside a DllMain(), you're not supposed to start threads from it.
-		 */
-		zend_init_timeout_thread();
+
+        /* Don't use ChangeTimerQueueTimer() as it will not restart an expired
+		timer, so we could end up with just an ignored timeout. Instead
+		delete and recreate. */
+	if (NULL != tq_timer) {
+		if (!DeleteTimerQueueTimer(NULL, tq_timer, NULL)) {
+			EG(timed_out) = 0;
+			tq_timer = NULL;
+			zend_error(E_ERROR, "Could not delete queued timer");
+			return;
+		}
+		tq_timer = NULL;
 	}
-	PostThreadMessage(timeout_thread_id, WM_REGISTER_ZEND_TIMEOUT, (WPARAM) GetCurrentThreadId(), (LPARAM) seconds);
+
+	/* XXX passing NULL means the default timer queue provided by the system is used */
+	if (!CreateTimerQueueTimer(&tq_timer, NULL, (WAITORTIMERCALLBACK)tq_timer_cb, (VOID*)&EG(timed_out), seconds*1000, 0, WT_EXECUTEONLYONCE)) {
+		EG(timed_out) = 0;
+		tq_timer = NULL;
+		zend_error(E_ERROR, "Could not queue new timer");
+		return;
+	}
+	EG(timed_out) = 0;
 #else
 #	ifdef HAVE_SETITIMER
 	{
@@ -1513,9 +1429,16 @@ void zend_set_timeout(long seconds, int reset_signals) /* {{{ */
 void zend_unset_timeout(TSRMLS_D) /* {{{ */
 {
 #ifdef ZEND_WIN32
-	if(timeout_thread_initialized) {
-		PostThreadMessage(timeout_thread_id, WM_UNREGISTER_ZEND_TIMEOUT, (WPARAM) GetCurrentThreadId(), (LPARAM) 0);
+	if (NULL != tq_timer) {
+		if (!DeleteTimerQueueTimer(NULL, tq_timer, NULL)) {
+			EG(timed_out) = 0;
+			tq_timer = NULL;
+			zend_error(E_ERROR, "Could not delete queued timer");
+			return;
+		}
+		tq_timer = NULL;
 	}
+	EG(timed_out) = 0;
 #else
 #	ifdef HAVE_SETITIMER
 	if (EG(timeout_seconds)) {