You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2020/07/08 12:07:38 UTC
svn commit: r1879645 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS
modules/core/mod_watchdog.c
Author: minfrin
Date: Wed Jul 8 12:07:38 2020
New Revision: 1879645
URL: http://svn.apache.org/viewvc?rev=1879645&view=rev
Log:
*) 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]
Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/modules/core/mod_watchdog.c
Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
Merged /httpd/httpd/trunk:r1876511
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1879645&r1=1879644&r2=1879645&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Wed Jul 8 12:07:38 2020
@@ -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]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1879645&r1=1879644&r2=1879645&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Wed Jul 8 12:07:38 2020
@@ -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:
Modified: httpd/httpd/branches/2.4.x/modules/core/mod_watchdog.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/core/mod_watchdog.c?rev=1879645&r1=1879644&r2=1879645&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/core/mod_watchdog.c (original)
+++ httpd/httpd/branches/2.4.x/modules/core/mod_watchdog.c Wed Jul 8 12:07:38 2020
@@ -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(vo
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(a
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_watchd
{
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_watchd
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;
}