You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tcl.apache.org by mx...@apache.org on 2020/04/16 21:33:22 UTC

[tcl-rivet] branch quattuor updated: * src/mod_rivet_ng/mod_rivet.[c|h]: add ap_child_shutdown flag * src/mod_rivet_ng/rivet_lazy_mpm.c: adding mutex protection on thread count * src/mod_rivet_ng/rivet_worker_mpm.c: removed old code

This is an automated email from the ASF dual-hosted git repository.

mxmanghi pushed a commit to branch quattuor
in repository https://gitbox.apache.org/repos/asf/tcl-rivet.git


The following commit(s) were added to refs/heads/quattuor by this push:
     new c971780      * src/mod_rivet_ng/mod_rivet.[c|h]: add ap_child_shutdown flag 	* src/mod_rivet_ng/rivet_lazy_mpm.c: adding mutex protection on thread count     * src/mod_rivet_ng/rivet_worker_mpm.c: removed old code
c971780 is described below

commit c971780b176669754537c12193db60a4b32d31b2
Author: Massimo Manghi <mx...@apache.org>
AuthorDate: Thu Apr 16 23:32:48 2020 +0200

        * src/mod_rivet_ng/mod_rivet.[c|h]: add ap_child_shutdown flag
    	* src/mod_rivet_ng/rivet_lazy_mpm.c: adding mutex protection on thread count
        * src/mod_rivet_ng/rivet_worker_mpm.c: removed old code
---
 ChangeLog                                |  5 +++
 src/mod_rivet_ng/mod_rivet.c             |  2 +-
 src/mod_rivet_ng/mod_rivet.h             |  1 +
 src/mod_rivet_ng/rivet_lazy_mpm.c        | 74 ++++++++++++++++++++------------
 src/mod_rivet_ng/rivet_worker_mpm.c      | 63 ---------------------------
 src/mod_rivet_ng/worker_prefork_common.c |  2 +-
 6 files changed, 54 insertions(+), 93 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3fb784f..fd7ff00 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-16 Massimo Manghi <mx...@apache.org>
+    * src/mod_rivet_ng/mod_rivet.[c|h]: add ap_child_shutdown flag
+	* src/mod_rivet_ng/rivet_lazy_mpm.c: adding mutex protection on thread count
+    * src/mod_rivet_ng/rivet_worker_mpm.c: removed old code
+
 2020-01-19 Massimo Manghi <mx...@apache.org>
     * doc/xml/commands.xml: remove entry for command incr0 (dropped in 3.0)
 
diff --git a/src/mod_rivet_ng/mod_rivet.c b/src/mod_rivet_ng/mod_rivet.c
index 36df39f..c67dd63 100644
--- a/src/mod_rivet_ng/mod_rivet.c
+++ b/src/mod_rivet_ng/mod_rivet.c
@@ -475,7 +475,7 @@ Rivet_ServerInit (apr_pool_t *pPool, apr_pool_t *pLog, apr_pool_t *pTemp, server
 
 apr_status_t Rivet_Finalize(void* data)
 {
-
+    module_globals->ap_child_shutdown = 1;
     RIVET_MPM_BRIDGE_CALL(child_finalize,data);
     apr_threadkey_private_delete (rivet_thread_key);
 
diff --git a/src/mod_rivet_ng/mod_rivet.h b/src/mod_rivet_ng/mod_rivet.h
index db0d593..04de062 100644
--- a/src/mod_rivet_ng/mod_rivet.h
+++ b/src/mod_rivet_ng/mod_rivet.h
@@ -207,6 +207,7 @@ typedef struct _mod_rivet_globals {
     apr_pool_t*             pool;               
     char*                   rivet_mpm_bridge;       /* name of the MPM bridge                   */
     server_rec*             server;                 /* default host server_rec obj              */
+    int                     ap_child_shutdown;      /* shutdown inited by the child pool cleanup */
     int                     vhosts_count;           /* Number of configured virtual host including 
                                                      * the root server thus it's supposed to be >= 1 */
 	char*				    default_handler;		/* Default request handler code             */
diff --git a/src/mod_rivet_ng/rivet_lazy_mpm.c b/src/mod_rivet_ng/rivet_lazy_mpm.c
index 8a0281c..e889880 100644
--- a/src/mod_rivet_ng/rivet_lazy_mpm.c
+++ b/src/mod_rivet_ng/rivet_lazy_mpm.c
@@ -64,7 +64,6 @@ typedef struct lazy_tcl_worker {
 /* virtual host thread queue descriptor */
 
 typedef struct vhost_iface {
-    int                 idle_threads_cnt;   /* idle threads for the virtual hosts       */
     int                 threads_count;      /* total number of running and idle threads */
     apr_thread_mutex_t* mutex;              /* mutex protecting 'array'                 */
     apr_array_header_t* array;              /* LIFO array of lazy_tcl_worker pointers   */
@@ -205,11 +204,15 @@ static void* APR_THREAD_FUNC request_processor (apr_thread_t *thd, void *data)
      * do...while loop controlled by private->keep_going  */
 
     idx = w->conf->idx;
+
+    apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
+    (module_globals->mpm->vhosts[idx].threads_count)++;
+    apr_thread_mutex_unlock(module_globals->mpm->vhosts[idx].mutex);
+
     apr_thread_mutex_lock(w->mutex);
     ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,w->server,"processor thread startup completed");
     do 
     {
-        module_globals->mpm->vhosts[idx].idle_threads_cnt++;
         while ((w->status != init) && (w->status != thread_exit)) {
             apr_thread_cond_wait(w->condition,w->mutex);
         } 
@@ -219,7 +222,6 @@ static void* APR_THREAD_FUNC request_processor (apr_thread_t *thd, void *data)
         }
 
         w->status = processing;
-        module_globals->mpm->vhosts[idx].idle_threads_cnt--;
 
         /* Content generation */
 
@@ -243,12 +245,15 @@ static void* APR_THREAD_FUNC request_processor (apr_thread_t *thd, void *data)
             apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
             *(lazy_tcl_worker **) apr_array_push(module_globals->mpm->vhosts[idx].array) = w;
             apr_thread_mutex_unlock(module_globals->mpm->vhosts[idx].mutex);
+        } else {
+            apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
+            (module_globals->mpm->vhosts[idx].threads_count)--;
+            apr_thread_mutex_unlock(module_globals->mpm->vhosts[idx].mutex);
         }
 
-    } while (private->ext->keep_going && !module_globals->mpm->server_shutdown);
+    } while (private->ext->keep_going);
     apr_thread_mutex_unlock(w->mutex);
     
-    ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,w->server,"processor thread orderly exit");
     Lazy_RunConfScript(private,w,child_exit);
 
     Rivet_ReleaseRunningScripts(private->ext->interp->scripts);
@@ -260,10 +265,7 @@ static void* APR_THREAD_FUNC request_processor (apr_thread_t *thd, void *data)
         Tcl_DeleteInterp(private->ext->interp->interp);
     }
 
-    apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
-    (module_globals->mpm->vhosts[idx].threads_count)--;
-    apr_thread_mutex_unlock(module_globals->mpm->vhosts[idx].mutex);
-
+    ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,w->server,"processor thread orderly exit");
     apr_thread_exit(thd,APR_SUCCESS);
     return NULL;
 }
@@ -326,8 +328,7 @@ void LazyBridge_ChildInit (apr_pool_t* pool, server_rec* server)
      *
      */
 
-    rv = apr_thread_mutex_create(&module_globals->mpm->mutex,
-                                  APR_THREAD_MUTEX_UNNESTED,pool);
+    rv = apr_thread_mutex_create(&module_globals->mpm->mutex,APR_THREAD_MUTEX_UNNESTED,pool);
     ap_assert(rv == APR_SUCCESS);
 
     /* the mpm->vhosts array is created with as many entries as the number of
@@ -355,7 +356,6 @@ void LazyBridge_ChildInit (apr_pool_t* pool, server_rec* server)
         array = apr_array_make(pool,0,sizeof(void*));
         ap_assert(array != NULL);
         module_globals->mpm->vhosts[vh].array = array;
-        module_globals->mpm->vhosts[vh].idle_threads_cnt = 0;
         module_globals->mpm->vhosts[vh].threads_count = 0;
     }
     module_globals->mpm->server_shutdown = 0;
@@ -402,7 +402,6 @@ int LazyBridge_Request (request_rec* r,rivet_req_ctype ctype)
     if (apr_is_empty_array(array))
     {
         w = create_worker(module_globals->pool,r->server);
-        (module_globals->mpm->vhosts[conf->idx].threads_count)++; 
     }
     else
     {
@@ -438,8 +437,8 @@ int LazyBridge_Request (request_rec* r,rivet_req_ctype ctype)
  */
 
 rivet_thread_interp* LazyBridge_Interp (rivet_thread_private* private,
-                                      rivet_server_conf*    conf,
-                                      rivet_thread_interp*  interp)
+                                        rivet_server_conf*    conf,
+                                        rivet_thread_interp*  interp)
 {
     if (interp != NULL) { private->ext->interp = interp; }
 
@@ -448,25 +447,40 @@ rivet_thread_interp* LazyBridge_Interp (rivet_thread_private* private,
 
 apr_status_t LazyBridge_Finalize (void* data)
 {
-    int vh;
+    int idx;
+    server_rec* server = (server_rec*) data;
     rivet_server_conf* conf = RIVET_SERVER_CONF(((server_rec*) data)->module_config);
    
-    module_globals->mpm->server_shutdown = 1;
-    for (vh = 0; vh < module_globals->vhosts_count; vh++)
+    for (idx = 0; idx < module_globals->vhosts_count; idx++)
     {
         int try;
         int count;
         apr_array_header_t* array;
         apr_thread_mutex_t* mutex;
 
-        mutex = module_globals->mpm->vhosts[vh].mutex;
-        array = module_globals->mpm->vhosts[vh].array;
+        mutex = module_globals->mpm->vhosts[idx].mutex;
+        array = module_globals->mpm->vhosts[idx].array;
+
+        /* we need to lock the vhost data mutex */
+
         apr_thread_mutex_lock(mutex);
+        count = module_globals->mpm->vhosts[idx].threads_count;
+        apr_thread_mutex_unlock(mutex);
         try = 0;
-        do {
+        while ((try++ < 3) && (count > 0)) {
 
-            count = module_globals->mpm->vhosts[vh].threads_count;
-            if (((conf->idx == vh) && (count == 1)) || (count == 0)) { break; } 
+            ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,server,"waiting for %d thread to exit",count);
+            if ((conf->idx == idx) && (count == 1)) { break; } 
+
+            /* if ap_child_shutdown is set the child exit was triggered
+             * by the apache framework and this function is running
+             * within a framework controlled thread. We don't
+             * have to check count to see if the current thread is actually
+             * the last thread remaining in the worker thread pool
+             */
+
+            if (!module_globals->ap_child_shutdown && 
+                (conf->idx == idx) && (count == 1)) { break; } 
 
             while (!apr_is_empty_array(array)) 
             {
@@ -479,13 +493,16 @@ apr_status_t LazyBridge_Finalize (void* data)
                 apr_thread_cond_signal(w->condition);
                 apr_thread_mutex_unlock(w->mutex);
             }
-            apr_sleep(10000);
 
-        } while ((try++ < 3));
-        apr_thread_mutex_unlock(mutex);
+            apr_thread_mutex_lock(mutex);
+            count = module_globals->mpm->vhosts[idx].threads_count;
+            apr_thread_mutex_unlock(mutex);
+
+            apr_sleep(1000);
+
+        }
     }
 
-    apr_threadkey_private_delete(rivet_thread_key);
     return APR_SUCCESS;
 }
 
@@ -528,9 +545,10 @@ int LazyBridge_ExitHandler(rivet_thread_private* private)
          * to exit and is sequence the whole process to shutdown 
          * by calling exit() */
      
+        module_globals->mpm->server_shutdown = 1;
         LazyBridge_Finalize(private->r->server);
 
-    } 
+    }
 
     return TCL_OK;
 }
diff --git a/src/mod_rivet_ng/rivet_worker_mpm.c b/src/mod_rivet_ng/rivet_worker_mpm.c
index 1d5716f..5b710cf 100644
--- a/src/mod_rivet_ng/rivet_worker_mpm.c
+++ b/src/mod_rivet_ng/rivet_worker_mpm.c
@@ -799,69 +799,6 @@ apr_status_t Worker_Bridge_Finalize (void* data)
     return OK;
 }
 
-#if 0
-apr_status_t Worker_Bridge_Finalize (void* data)
-{
-    apr_status_t  rv;
-    apr_status_t  thread_status;
-    server_rec* s = (server_rec*) data;
-    
-    apr_thread_mutex_lock(module_globals->mpm->job_mutex);
-    module_globals->mpm->server_shutdown = 1;
-
-    /* We wake up the supervisor who is now supposed to stop 
-     * all the Tcl worker threads
-     */
-
-    apr_thread_cond_signal(module_globals->mpm->job_cond);
-    apr_thread_mutex_unlock(module_globals->mpm->job_mutex);
-
-    /* If the function is called by the memory pool cleanup we wait
-     * to join the supervisor, otherwise we if the function was called
-     * by Worker_Bridge_Exit we skip it because this thread itself must exit
-     * to allow the supervisor to exit in the shortest possible time 
-     */
-
-    if (!module_globals->mpm->exit_command)
-    {
-        rv = apr_thread_join (&thread_status,module_globals->mpm->supervisor);
-        if (rv != APR_SUCCESS)
-        {
-            ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
-                         MODNAME ": Error joining supervisor thread");
-        }
-    }
-
-    apr_threadkey_private_delete (rivet_thread_key);
-    return OK;
-}
-#endif
-
-/*
- * -- MPM_MasterInterp
- *
- *  Arguments:
- *
- *      server_rec* server: a server_rec pointer
- *
- *  Results:
- *
- */
-#if 0
-rivet_thread_interp* MPM_MasterInterp(server_rec* s)
-{
-    rivet_thread_private*   private;
-    rivet_thread_interp*    interp_obj; 
-
-    RIVET_PRIVATE_DATA_NOT_NULL(rivet_thread_key,private)
-
-    interp_obj = Rivet_NewVHostInterp(private,s);
-    interp_obj->channel = private->channel;
-    Rivet_PerInterpInit(interp_obj, private, s, private->pool);
-    return interp_obj;
-}
-#endif
-
 /*
  * -- Worker_Bridge_ExitHandler
  *  
diff --git a/src/mod_rivet_ng/worker_prefork_common.c b/src/mod_rivet_ng/worker_prefork_common.c
index 1aff536..7972430 100644
--- a/src/mod_rivet_ng/worker_prefork_common.c
+++ b/src/mod_rivet_ng/worker_prefork_common.c
@@ -180,7 +180,7 @@ void Rivet_ProcessorCleanup (rivet_thread_private* private)
      * the channel is released immediately by forcing the refCount to 0
      * (see Tcl source code: generic/TclIO.c). Unregistering for each interpreter
      * causes the process to segfault at least for certain Tcl versions.
-     * We unset the channel as stdout to avoid this
+     * In order to avoid the crash the channel is unset as stdout
      */
 
     Tcl_SetStdChannel(NULL,TCL_STDOUT);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tcl.apache.org
For additional commands, e-mail: commits-help@tcl.apache.org