Skip to content

Commit

Permalink
Use real lock around interpreter destruction to avoid potential race …
Browse files Browse the repository at this point in the history
…condition with deadlock detection thread.
  • Loading branch information
GrahamDumpleton committed Nov 14, 2017
1 parent 8e57570 commit 2748873
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 2 deletions.
8 changes: 8 additions & 0 deletions docs/release-notes/version-4.5.21.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ Bugs Fixed
deploying mod_wsgi to MacOS X. You need to use the ``pip install``
method.

* Speculated that crashes on daemon process shutdown were being caused
by a race condition around accessing Python C API when interpreter
was being destroyed. There was a check in place to avoid this but may
not have been robust enough depending on how memory cache worked
for threads running across multi core machine. Now use a dedicated
thread mutex to avoid race condition between main process thread and
Python interpreter deadlock detection thread.

Features Changed
----------------

Expand Down
7 changes: 6 additions & 1 deletion src/server/mod_wsgi.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static apr_array_header_t *wsgi_daemon_list = NULL;

static apr_pool_t *wsgi_parent_pool = NULL;

static int volatile wsgi_daemon_shutdown = 0;
int volatile wsgi_daemon_shutdown = 0;
static int volatile wsgi_daemon_graceful = 0;

#if defined(MOD_WSGI_WITH_DAEMONS)
Expand Down Expand Up @@ -4288,6 +4288,7 @@ static void wsgi_python_child_init(apr_pool_t *p)
#if APR_HAS_THREADS
apr_thread_mutex_create(&wsgi_interp_lock, APR_THREAD_MUTEX_UNNESTED, p);
apr_thread_mutex_create(&wsgi_module_lock, APR_THREAD_MUTEX_UNNESTED, p);
apr_thread_mutex_create(&wsgi_shutdown_lock, APR_THREAD_MUTEX_UNNESTED, p);
#endif

/*
Expand Down Expand Up @@ -8954,11 +8955,15 @@ static void *wsgi_deadlock_thread(apr_thread_t *thd, void *data)
while (1) {
apr_sleep(apr_time_from_sec(1));

apr_thread_mutex_lock(wsgi_shutdown_lock);

if (!wsgi_daemon_shutdown) {
gilstate = PyGILState_Ensure();
PyGILState_Release(gilstate);
}

apr_thread_mutex_unlock(wsgi_shutdown_lock);

apr_thread_mutex_lock(wsgi_monitor_lock);
wsgi_deadlock_shutdown_time = apr_time_now();
wsgi_deadlock_shutdown_time += wsgi_deadlock_timeout;
Expand Down
2 changes: 2 additions & 0 deletions src/server/wsgi_daemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ extern WSGIDaemonThread *wsgi_worker_threads;

extern WSGIThreadStack *wsgi_worker_stack;

extern int volatile wsgi_daemon_shutdown;

#endif

/* ------------------------------------------------------------------------- */
Expand Down
15 changes: 14 additions & 1 deletion src/server/wsgi_interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1986,10 +1986,22 @@ apr_status_t wsgi_python_term(void)
if (!PyImport_AddModule("dummy_threading"))
PyErr_Clear();

/* Shutdown Python interpreter completely. */
/*
* Shutdown Python interpreter completely. Just to be safe
* flag daemon shutdown here again and do it within a lock
* which is then shared with deadlock thread used for the
* daemon. This is just to avoid any risk there is a race
* condition.
*/

apr_thread_mutex_lock(wsgi_shutdown_lock);

wsgi_daemon_shutdown++;

Py_Finalize();

apr_thread_mutex_unlock(wsgi_shutdown_lock);

wsgi_python_initialized = 0;

ap_log_error(APLOG_MARK, APLOG_INFO, 0, wsgi_server,
Expand Down Expand Up @@ -2336,6 +2348,7 @@ void wsgi_python_init(apr_pool_t *p)

#if APR_HAS_THREADS
apr_thread_mutex_t* wsgi_interp_lock = NULL;
apr_thread_mutex_t* wsgi_shutdown_lock = NULL;
#endif

PyObject *wsgi_interpreters = NULL;
Expand Down
1 change: 1 addition & 0 deletions src/server/wsgi_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ extern PyObject *wsgi_interpreters;

#if APR_HAS_THREADS
extern apr_thread_mutex_t *wsgi_interp_lock;
extern apr_thread_mutex_t* wsgi_shutdown_lock;
#endif

extern void wsgi_python_version(void);
Expand Down

0 comments on commit 2748873

Please sign in to comment.