From: Evgeny Kotkov <kotkov@apache.org>
Date: Tue, 11 Jul 2017 16:46:47 +0000 (+0000)
Subject: mpm_winnt: Avoid using TerminateThread() in case the shutdown routine
X-Git-Tag: 2.5.0-alpha~294
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5c1f5c7b2824da01840f234c25b06330783357fa;p=apache

mpm_winnt: Avoid using TerminateThread() in case the shutdown routine
hits a timeout while waiting for the worker threads to exit.

Using TerminateThread() can have dangerous consequences such as deadlocks —
say, if the the thread is terminated while holding a lock or a heap lock
in the middle of HeapAlloc(), as these locks would not be released.
Or it can corrupt the application state and cause a crash.

(See https://msdn.microsoft.com/en-us/library/windows/desktop/ms686717)

Rework the code to call TerminateProcess() in the described circumstances
and leave the cleanup to the operating system.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1801636 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/server/mpm/winnt/child.c b/server/mpm/winnt/child.c
index 16407e6bca..e2d64c50c4 100644
--- a/server/mpm/winnt/child.c
+++ b/server/mpm/winnt/child.c
@@ -1260,21 +1260,30 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
         }
     }
 
-    /* Kill remaining threads off the hard way */
     if (threads_created) {
         ap_log_error(APLOG_MARK, APLOG_NOTICE, APR_SUCCESS, ap_server_conf, APLOGNO(00363)
-                     "Child: Terminating %d threads that failed to exit.",
+                     "Child: Waiting for %d threads timed out, terminating process.",
                      threads_created);
-    }
-    for (i = 0; i < threads_created; i++) {
-        int *idx;
-        TerminateThread(child_handles[i], 1);
-        CloseHandle(child_handles[i]);
-        /* Reset the scoreboard entry for the thread we just whacked */
-        idx = apr_hash_get(ht, &child_handles[i], sizeof(HANDLE));
-        if (idx) {
-            ap_update_child_status_from_indexes(0, *idx, SERVER_DEAD, NULL);
+        for (i = 0; i < threads_created; i++) {
+            /* Reset the scoreboard entries for the threads. */
+            int *idx = apr_hash_get(ht, &child_handles[i], sizeof(HANDLE));
+            if (idx) {
+                ap_update_child_status_from_indexes(0, *idx, SERVER_DEAD, NULL);
+            }
         }
+        /* We can't wait for any longer, but still have some threads remaining.
+         *
+         * The only thing we can do is terminate the process and let the OS
+         * perform the required cleanup. Terminate with exit code 0, as we do
+         * not want the parent process to restart the child. Note that we use
+         * TerminateProcess() instead of ExitProcess(), as the latter function
+         * causes all DLLs to be unloaded, and it can potentially deadlock if
+         * a DLL unload handler tries to acquire the same lock that is being
+         * held by one of the remaining worker threads.
+         *
+         * See https://msdn.microsoft.com/en-us/library/windows/desktop/ms682658
+         */
+        TerminateProcess(GetCurrentProcess(), 0);
     }
     ap_log_error(APLOG_MARK, APLOG_NOTICE, APR_SUCCESS, ap_server_conf, APLOGNO(00364)
                  "Child: All worker threads have exited.");