You are viewing a plain text version of this content. The canonical link for it is here.
Posted to site-cvs@tcl.apache.org by mx...@apache.org on 2015/06/28 15:29:07 UTC

svn commit: r1688005 - in /tcl/rivet/trunk: ChangeLog src/mod_rivet/mod_rivet.c src/mod_rivet/mod_rivet.h src/mod_rivet/rivet_worker_mpm.c

Author: mxmanghi
Date: Sun Jun 28 13:29:06 2015
New Revision: 1688005

URL: http://svn.apache.org/r1688005
Log:
    * src/mod_rivet/mod_rivet.c: fixed errors in comments
    * src/mod_rivet/rivet_worker_mpm.c: Binding release of mutex and
    condition variable to MPM thread pool cleanup, hopefully fixing
    this potential memory leak


Modified:
    tcl/rivet/trunk/ChangeLog
    tcl/rivet/trunk/src/mod_rivet/mod_rivet.c
    tcl/rivet/trunk/src/mod_rivet/mod_rivet.h
    tcl/rivet/trunk/src/mod_rivet/rivet_worker_mpm.c

Modified: tcl/rivet/trunk/ChangeLog
URL: http://svn.apache.org/viewvc/tcl/rivet/trunk/ChangeLog?rev=1688005&r1=1688004&r2=1688005&view=diff
==============================================================================
--- tcl/rivet/trunk/ChangeLog (original)
+++ tcl/rivet/trunk/ChangeLog Sun Jun 28 13:29:06 2015
@@ -1,3 +1,9 @@
+2015-06-28 Massimo Manghi <mx...@apache.org>
+    * src/mod_rivet/mod_rivet.c: fixed errors in comments
+    * src/mod_rivet/rivet_worker_mpm.c: Binding release of mutex and
+    condition variable to MPM thread pool cleanup, hopefully fixing
+    this potential memory leak
+
 2015-06-21 Massimo Manghi <mx...@apache.org>
     * configure.ac: add creation of config variable RIVET_CONFIGURE_CMD
     * src/mod_rivet/mod_rivet_common.c: add RIVET_CONFIGURE_CMD handling

Modified: tcl/rivet/trunk/src/mod_rivet/mod_rivet.c
URL: http://svn.apache.org/viewvc/tcl/rivet/trunk/src/mod_rivet/mod_rivet.c?rev=1688005&r1=1688004&r2=1688005&view=diff
==============================================================================
--- tcl/rivet/trunk/src/mod_rivet/mod_rivet.c (original)
+++ tcl/rivet/trunk/src/mod_rivet/mod_rivet.c Sun Jun 28 13:29:06 2015
@@ -455,12 +455,12 @@ void Rivet_ProcessorCleanup (void *data)
                                         private->req_cnt,module_globals->vhosts_count);
 
     /* We are deleting the interpreters and release the thread channel. 
-     * Rivet channel is set a stdout channel of Tcl and as such is treated
+     * Rivet channel is set as stdout channel of Tcl and as such is treated
      * by Tcl_UnregisterChannel is a special way. When its refCount reaches 1
-     * the channel is released immediatly by forcing the refCount to 0
+     * 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 stdout channel to avoid this
+     * We unset the channel as stdout to avoid this
      */
 
     Tcl_SetStdChannel(NULL,TCL_STDOUT);
@@ -487,8 +487,6 @@ void Rivet_ProcessorCleanup (void *data)
         if ((i > 0) && rsc->separate_channels) 
             Rivet_ReleaseRivetChannel(private->interps[i]->interp,private->channel);
 
-        //Tcl_UnregisterChannel(private->interps[i]->interp,*private->channel);       
-
         Tcl_DeleteInterp(private->interps[i]->interp);
 
         /* if separate_virtual_interps == 0 we are running the same interpreter

Modified: tcl/rivet/trunk/src/mod_rivet/mod_rivet.h
URL: http://svn.apache.org/viewvc/tcl/rivet/trunk/src/mod_rivet/mod_rivet.h?rev=1688005&r1=1688004&r2=1688005&view=diff
==============================================================================
--- tcl/rivet/trunk/src/mod_rivet/mod_rivet.h (original)
+++ tcl/rivet/trunk/src/mod_rivet/mod_rivet.h Sun Jun 28 13:29:06 2015
@@ -168,12 +168,12 @@ typedef struct mpm_bridge_status mpm_bri
 
 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  */
+    char*               rivet_mpm_bridge;       /* name of the MPM bridge               */
+    server_rec*         server;                 /* default host server_rec obj          */
     int                 vhosts_count;           /* Number of configured virtual host including 
                                                  * the root server thus it's supposed to be >= 1 */
     rivet_thread_interp* 
-                        server_interp;          /* server and prefork MPM interpreter */
+                        server_interp;          /* server and prefork MPM interpreter   */
     apr_thread_mutex_t* pool_mutex;             /* threads commmon pool mutex           */
 
     /* Jump table to bridge specific procedures */

Modified: tcl/rivet/trunk/src/mod_rivet/rivet_worker_mpm.c
URL: http://svn.apache.org/viewvc/tcl/rivet/trunk/src/mod_rivet/rivet_worker_mpm.c?rev=1688005&r1=1688004&r2=1688005&view=diff
==============================================================================
--- tcl/rivet/trunk/src/mod_rivet/rivet_worker_mpm.c (original)
+++ tcl/rivet/trunk/src/mod_rivet/rivet_worker_mpm.c Sun Jun 28 13:29:06 2015
@@ -27,6 +27,7 @@
 #include <ap_mpm.h>
 #include <apr_strings.h>
 
+#include "rivet.h"
 #include "mod_rivet.h"
 #include "mod_rivet_common.h"
 #include "httpd.h"
@@ -379,6 +380,19 @@ int Rivet_MPM_ServerInit (apr_pool_t *pP
     return OK;
 }
 
+/*
+ * -- Rivet_MPM_ChildInit
+ *
+ * Child initialization function called by the web server framework.
+ * For this bridge tasks are 
+ *
+ *   + mpm_bridge_status object allocation and initialization
+ *   + content generation callback private key creation
+ *   + supervisor thread creation
+ * 
+ *
+ */
+
 void Rivet_MPM_ChildInit (apr_pool_t* pool, server_rec* server)
 {
     apr_status_t        rv;
@@ -453,13 +467,39 @@ void Rivet_MPM_ChildInit (apr_pool_t* po
         char    errorbuf[512];
 
         apr_strerror(rv, errorbuf,200);
-        ap_log_error(APLOG_MARK, APLOG_ERR, APR_EGENERAL, server, 
+        ap_log_error(APLOG_MARK, APLOG_ERR, rv, server, 
                      MODNAME "Error starting supervisor thread rv=%d:%s\n",rv,errorbuf);
         exit(1);
     }
 }
 
 /*
+ * -- Worker_PrivateCleanup
+ *
+ *  Pool cleanup function registered with the MPM controlled thread. Purpose
+ * of this function is to properly get rid of data allocated on the
+ * handler_private object
+ *
+ * Arguments:
+ *
+ *  void* client_data: a generic pointer cast to a handler_private object pointer
+ *
+ */ 
+
+apr_status_t Worker_RequestPrivateCleanup (void *client_data)
+{
+    handler_private* req_private = (handler_private*)client_data;
+
+    apr_thread_cond_destroy(req_private->cond);
+    apr_thread_mutex_destroy(req_private->mutex);
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, module_globals->server,
+                 MODNAME ": request thread private data released");
+
+    return APR_SUCCESS;
+}
+
+
+/*
  * -- Rivet_MPM_Request
  *
  * Content generation callback. Actually on the bridge this function is not
@@ -490,38 +530,38 @@ int Rivet_MPM_Request (request_rec* r)
 
     if (apr_threadkey_private_get ((void **)&request_private,handler_thread_key) != APR_SUCCESS)
     {
-        ap_log_error(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r->server,
-                     MODNAME ": cannot get private data for processor thread");
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, MODNAME ": cannot get private data for processor thread");
 
         return HTTP_INTERNAL_SERVER_ERROR;
 
     } else {
 
-        /* Checking if we are at the first request server by this thread */
+        /* if the thread is serving its first request we allocate its private data from the thread pool */
 
         if (request_private == NULL)
         {
             apr_pool_t *tpool = apr_thread_pool_get(r->connection->current_thread);
 
-
-            //apr_thread_mutex_lock(module_globals->pool_mutex);
-            request_private      = apr_palloc(tpool,sizeof(handler_private));
+            request_private = apr_palloc(tpool,sizeof(handler_private));
  
-            apr_thread_cond_create(&(request_private->cond), tpool);
-            apr_thread_mutex_create(&(request_private->mutex), APR_THREAD_MUTEX_UNNESTED, tpool);
-            //apr_thread_mutex_unlock(module_globals->pool_mutex);
-
+            apr_thread_cond_create (&(request_private->cond), tpool);
+            apr_thread_mutex_create (&(request_private->mutex), APR_THREAD_MUTEX_UNNESTED, tpool);
             apr_threadkey_private_set (request_private,handler_thread_key);
+
+            apr_pool_cleanup_register(tpool,(void *)request_private,Worker_RequestPrivateCleanup,apr_pool_cleanup_null);
+
+            ap_log_rerror(APLOG_MARK, APLOG_INFO, APR_SUCCESS, r,  MODNAME ": request thread private data allocated");
+
         }
 
     }
 
     /* We prepare the request descriptor object to be placed on the Tcl working threads queue */
 
-    request_private->r      = r;
-    request_private->code   = OK;
-    request_private->status = init;
-    request_private->job_type = request;
+    request_private->r          = r;
+    request_private->code       = OK;
+    request_private->status     = init;
+    request_private->job_type   = request;
 
     rv = apr_queue_push(module_globals->mpm->queue,request_private);
     if (rv == APR_SUCCESS)
@@ -584,8 +624,9 @@ rivet_thread_interp* Rivet_MPM_MasterInt
     rivet_thread_private*   private;
     rivet_thread_interp*           interp_obj; 
 
-    ap_assert (apr_threadkey_private_get ((void **)&private,rivet_thread_key) == APR_SUCCESS);
-    ap_assert (private != NULL);
+    RIVET_PRIVATE_DATA_NOT_NULL(rivet_thread_key,private)
+    //ap_assert (apr_threadkey_private_get ((void **)&private,rivet_thread_key) == APR_SUCCESS);
+    //ap_assert (private != NULL);
 
     interp_obj = Rivet_NewVHostInterp(private->pool);
     //interp_obj->channel = Rivet_CreateRivetChannel(private->pool,rivet_thread_key);



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