Don't destroy Python interpreters on process shutdown.

7 files changed, 136 insertions, 0 deletions

diff --git a/docs/release-notes.rst b/docs/release-notes.rst
index 3df2288..fa6d30c 100644
--- a/docs/release-notes.rst
+++ b/docs/release-notes.rst
@@ -5,6 +5,7 @@ Release Notes
 .. toctree::
    :maxdepth: 2
 
+   release-notes/version-4.9.1
    release-notes/version-4.9.0
 
    release-notes/version-4.8.0
diff --git a/docs/release-notes/version-4.9.1.rst b/docs/release-notes/version-4.9.1.rst
new file mode 100644
index 0000000..7177274
--- /dev/null
+++ b/docs/release-notes/version-4.9.1.rst
@@ -0,0 +1,84 @@
+=============
+Version 4.9.1
+=============
+
+Version 4.9.1 of mod_wsgi can be obtained from:
+
+  https://codeload.github.com/GrahamDumpleton/mod_wsgi/tar.gz/4.9.1
+
+Features Changed
+----------------
+
+* Historically when a process was being shutdown, mod_wsgi would do its best to
+  destroy any Python sub interpreters as well as the main Python interpreter.
+  This was done in case applications attempted to run any actions on process
+  shutdown via ``atexit`` registered callbacks or other means.
+
+  Because of changes in Python 3.9, and possibly because mod_wsgi makes use of
+  externally created C threads to handle requests, and not Python native
+  threads, there is now a possibility that attempting to delete Python sub
+  interpreters will hang. It is believed this may relate to Python core now
+  expecting all Python thread state objects to have been deleted before the
+  Python sub interpreter can be destroyed. If they aren't then Python core
+  code can block indefinitely. If the issue isn't the externally created C
+  threads that mod_wsgi uses, it might instead be arising as a problem when a
+  hosted WSGI application creates its own background threads but they are
+  still running when the attempt is made to destroy the sub interpreter.
+
+  In the case of using daemon mode the result is that processes can hang on
+  shutdown, but will still at least be deleted after 5 seconds due to how
+  Apache process management will forcibly kill managed processes after 5
+  seconds if they do not exit cleanly themselves. In other words the issue
+  may not be noticed.
+
+  For embedded mode however, the Apache child process can hang around
+  indefinitely, possibly only being deleted if some higher level system
+  application manager such as systemd is able to detect the problem and
+  forcibly deleted the hung process.
+
+  Although mod_wsgi always attempts to ensure that the externally created C
+  threads are not still handling HTTP requests and thus not active prior to
+  destroying the Python interpreter, it is impossible to guarantee this.
+  Similarly, there is no way to guarantee that background threads created by a
+  WSGI application aren't still running. As such, it isn't possible to safely
+  attempt to delete the Python thread state objects before deleting the Python
+  sub interpreter.
+
+  Because of this, from this version of mod_wsgi onwards, there will be no
+  attempt to destroy the Python sub interpreters or the main Python
+  interpreter when the process is being shutdown. As this means that
+  ``atexit`` registered callbacks will no longer be called, it is important
+  that you use mod_wsgi's own mechanism of being notified when a process is
+  being shutdown to perform any special actions.
+
+  ::
+
+    import mod_wsgi
+
+    def shutdown_handler(event, **kwargs):
+      print('SHUTDOWN-HANDLER', event, kwargs)
+
+    mod_wsgi.subscribe_shutdown(shutdown_handler)
+  
+  Use of this shutdown notification was necessary anyway to reliably attempt
+  to stop background threads created by the WSGI application since ``atexit``
+  registered callbacks are not called by Python core until after it thinks all
+  threads have been stopped. In other words, ``atexit`` register callbacks
+  couldn't be used to reliably stop background threads. Thus use of the
+  mod_wsgi mechanism for performing actions on process shutdown is the
+  preferred way.
+
+  Overall it is expected that the majority of users will not notice this
+  change as it is very rare to see WSGI applications want to perform special
+  actions on process shutdown. If you are affected, you should use mod_wsgi's
+  mechanism to perform special actions on process shutdown.
+
+  If for some reason you want to revert to the prior behavior and have
+  mod_wsgi attempt to destroy any Python sub interpreters and the main Python
+  interpreter on process shutdown and you are manually configuring Apache, you
+  can add at global scope in the Apache configuration::
+
+    WSGIDestroyInterpreter On
+
+  If you are using mod_wsgi-express, you can instead supply the command line
+  option ``--destroy-interpreter``.
\ No newline at end of file
diff --git a/src/server/__init__.py b/src/server/__init__.py
index 5702801..54c36cb 100644
--- a/src/server/__init__.py
+++ b/src/server/__init__.py
@@ -353,6 +353,13 @@ WSGISocketRotation Off
 MaxConnectionsPerChild %(maximum_requests)s
 </IfDefine>
 
+<IfDefine DESTROY_INTERPRETER>
+WSGIDestroyInterpreter On
+</IfDefine>
+<IfDefine !DESTROY_INTERPRETER>
+WSGIDestroyInterpreter Off
+</IfDefine>
+
 <IfDefine !ONE_PROCESS>
 <IfDefine !EMBEDDED_MODE>
 WSGIRestrictEmbedded On
@@ -2692,6 +2699,10 @@ add_option('unix', '--service-log-file', action='append', nargs=2,
         help='Specify the name of a separate log file to be used for '
         'the managed service.')
 
+add_option('all', '--destroy-interpreter', action='store_true',
+        default=False, help='Flag indicating whether the Python '
+        'interpreter should be destroyed on process shutdown.')
+
 add_option('unix', '--embedded-mode', action='store_true', default=False,
         help='Flag indicating whether to run in embedded mode rather '
         'than the default daemon mode. Numerous daemon mode specific '
@@ -3348,6 +3359,9 @@ def _cmd_setup_server(command, args, options):
     else:
         options['https_url'] = None
 
+    if options['destroy_interpreter']:
+        options['httpd_arguments_list'].append('-DDESTROY_INTERPRETER')
+
     if options['embedded_mode']:
         options['httpd_arguments_list'].append('-DEMBEDDED_MODE')
         options['disable_reloading'] = True
diff --git a/src/server/mod_wsgi.c b/src/server/mod_wsgi.c
index 9bef2bd..f91af5e 100644
--- a/src/server/mod_wsgi.c
+++ b/src/server/mod_wsgi.c
@@ -4306,6 +4306,11 @@ static apr_status_t wsgi_python_child_cleanup(void *data)
     wsgi_publish_process_stopping(wsgi_shutdown_reason);
 #endif
 
+    /* Skip destruction of Python interpreter. */
+
+    if (wsgi_server_config->destroy_interpreter != 1)
+        return APR_SUCCESS;
+
     /* In a multithreaded MPM must protect table. */
 
 #if APR_HAS_THREADS
@@ -5043,6 +5048,28 @@ static const char *wsgi_set_python_hash_seed(cmd_parms *cmd, void *mconfig,
     return NULL;
 }
 
+static const char *wsgi_set_destroy_interpreter(cmd_parms *cmd, void *mconfig,
+                                                const char *f)
+{
+    const char *error = NULL;
+    WSGIServerConfig *sconfig = NULL;
+
+    error = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+    if (error != NULL)
+        return error;
+
+    sconfig = ap_get_module_config(cmd->server->module_config, &wsgi_module);
+
+    if (strcasecmp(f, "Off") == 0)
+        sconfig->destroy_interpreter = 0;
+    else if (strcasecmp(f, "On") == 0)
+        sconfig->destroy_interpreter = 1;
+    else
+        return "WSGIDestroyInterpreter must be one of: Off | On";
+
+    return NULL;
+}
+
 static const char *wsgi_set_restrict_embedded(cmd_parms *cmd, void *mconfig,
                                               const char *f)
 {
@@ -16224,6 +16251,9 @@ static const command_rec wsgi_commands[] =
     AP_INIT_TAKE1("WSGIPythonHashSeed", wsgi_set_python_hash_seed,
         NULL, RSRC_CONF, "Python hash seed."),
 
+    AP_INIT_TAKE1("WSGIDestroyInterpreter", wsgi_set_destroy_interpreter,
+        NULL, RSRC_CONF, "Enable/Disable destruction of Python interpreter."),
+
 #if defined(MOD_WSGI_WITH_DAEMONS)
     AP_INIT_TAKE1("WSGIRestrictEmbedded", wsgi_set_restrict_embedded,
         NULL, RSRC_CONF, "Enable/Disable use of embedded mode."),
diff --git a/src/server/wsgi_interp.c b/src/server/wsgi_interp.c
index 027325f..60c58aa 100644
--- a/src/server/wsgi_interp.c
+++ b/src/server/wsgi_interp.c
@@ -2069,6 +2069,11 @@ apr_status_t wsgi_python_term(void)
 {
     PyObject *module = NULL;
 
+    /* Skip destruction of Python interpreter. */
+
+    if (wsgi_server_config->destroy_interpreter != 1)
+        return APR_SUCCESS;
+
     ap_log_error(APLOG_MARK, APLOG_INFO, 0, wsgi_server,
                  "mod_wsgi (pid=%d): Terminating Python.", getpid());
 
diff --git a/src/server/wsgi_server.c b/src/server/wsgi_server.c
index 2954d4c..050f5c7 100644
--- a/src/server/wsgi_server.c
+++ b/src/server/wsgi_server.c
@@ -101,6 +101,7 @@ WSGIServerConfig *newWSGIServerConfig(apr_pool_t *p)
 
     object->python_hash_seed = NULL;
 
+    object->destroy_interpreter = -1;
     object->restrict_embedded = -1;
     object->restrict_stdin = -1;
     object->restrict_stdout = -1;
diff --git a/src/server/wsgi_server.h b/src/server/wsgi_server.h
index c433959..49c9dca 100644
--- a/src/server/wsgi_server.h
+++ b/src/server/wsgi_server.h
@@ -87,6 +87,7 @@ typedef struct {
 
     const char *python_hash_seed;
 
+    int destroy_interpreter;
     int restrict_embedded;
     int restrict_stdin;
     int restrict_stdout;