*) mod_watchdog: Switch to simpler logic to avoid the thread cleanup running

     before the thread has started, avoiding mutex operations with undefined
     behaviour. [Christophe Jaillet]

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1879645 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 31ad67417151a9f56630f9b5c2a91827d90ce47f..b6d0f476459a84ce6fdf8f3d38d8ffc8ae615743 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.44
 
+  *) mod_watchdog: Switch to simpler logic to avoid the thread cleanup running
+     before the thread has started, avoiding mutex operations with undefined
+     behaviour. [Christophe Jaillet]
+
   *) mod_http2: The module now handles master/secondary connections and has marked
      methods according to use. [Stefan Eissing]
 

diff --git a/STATUS b/STATUS
index e558da08eab327954f346387a5e70ea9f4ca2fc4..566a0189e1248e4c336e1cae429237aa17d4a3cb 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -135,11 +135,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_watchdog: Switch to simpler logic to avoid the thread cleanup running
-     before the thread has started, avoiding mutex operations with undefined behaviour
-     trunk patch: http://svn.apache.org/r1876511
-     2.4.x patch: svn merge -c 1876511 ^/httpd/httpd/trunk .
-     +1: jailletc36 (by inspection), jim, minfrin
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/core/mod_watchdog.c b/modules/core/mod_watchdog.c
index 51dea91c69e7f6689945639139c45dc7dc1d8e8e..bc05e8521150b5f07b4dc5456a140dc3710289f3 100644 (file)
--- a/modules/core/mod_watchdog.c
+++ b/modules/core/mod_watchdog.c
@@ -24,6 +24,8 @@
 #include "http_core.h"
 #include "util_mutex.h"
 
+#include "apr_atomic.h"
+
 #define AP_WATCHDOG_PGROUP    "watchdog"
 #define AP_WATCHDOG_PVERSION  "parent"
 #define AP_WATCHDOG_CVERSION  "child"
@@ -43,7 +45,7 @@ struct watchdog_list_t
 
 struct ap_watchdog_t
 {
-    apr_thread_mutex_t   *startup;
+    apr_uint32_t          thread_started; /* set to 1 once thread running */
     apr_proc_mutex_t     *mutex;
     const char           *name;
     watchdog_list_t      *callbacks;
@@ -74,6 +76,10 @@ static apr_status_t wd_worker_cleanup(void *data)
     apr_status_t rv;
     ap_watchdog_t *w = (ap_watchdog_t *)data;
 
+    /* Do nothing if the thread wasn't started. */
+    if (apr_atomic_read32(&w->thread_started) != 1)
+        return APR_SUCCESS;
+
     if (w->is_running) {
         watchdog_list_t *wl = w->callbacks;
         while (wl) {
@@ -110,7 +116,8 @@ static void* APR_THREAD_FUNC wd_worker(apr_thread_t *thread, void *data)
     w->pool = apr_thread_pool_get(thread);
     w->is_running = 1;
 
-    apr_thread_mutex_unlock(w->startup);
+    apr_atomic_set32(&w->thread_started, 1); /* thread started */
+
     if (w->mutex) {
         while (w->is_running) {
             if (ap_mpm_query(AP_MPMQ_MPM_STATE, &mpmq_s) != APR_SUCCESS) {
@@ -264,10 +271,7 @@ static apr_status_t wd_startup(ap_watchdog_t *w, apr_pool_t *p)
 {
     apr_status_t rc;
 
-    /* Create thread startup mutex */
-    rc = apr_thread_mutex_create(&w->startup, APR_THREAD_MUTEX_UNNESTED, p);
-    if (rc != APR_SUCCESS)
-        return rc;
+    apr_atomic_set32(&w->thread_started, 0);
 
     if (w->singleton) {
         /* Initialize singleton mutex in child */
@@ -277,22 +281,12 @@ static apr_status_t wd_startup(ap_watchdog_t *w, apr_pool_t *p)
             return rc;
     }
 
-    /* This mutex fixes problems with a fast start/fast end, where the pool
-     * cleanup was being invoked before the thread completely spawned.
-     */
-    apr_thread_mutex_lock(w->startup);
-    apr_pool_pre_cleanup_register(p, w, wd_worker_cleanup);
-
     /* Start the newly created watchdog */
     rc = apr_thread_create(&w->thread, NULL, wd_worker, w, p);
-    if (rc) {
-        apr_pool_cleanup_kill(p, w, wd_worker_cleanup);
+    if (rc == APR_SUCCESS) {
+        apr_pool_pre_cleanup_register(p, w, wd_worker_cleanup);
     }
 
-    apr_thread_mutex_lock(w->startup);
-    apr_thread_mutex_unlock(w->startup);
-    apr_thread_mutex_destroy(w->startup);
-
     return rc;
 }