svn commit: r1482133 - in /subversion/branches/1.8.x: ./ STATUS subversion/svnserve/svnserve.c

[breser]

Author: breser
Date: Mon May 13 22:32:01 2013
New Revision: 1482133

URL: http://svn.apache.org/r1482133
Log:
Merge r1481010 from trunk:

 * r1481010
   Fix issue 4365, multi-threaded svnserve pool memory use after clear.
   Justification:
     SEGV, particularly on Windows.
   Votes:
     +1: philip, danielsh, ivan


Modified:
    subversion/branches/1.8.x/   (props changed)
    subversion/branches/1.8.x/STATUS
    subversion/branches/1.8.x/subversion/svnserve/svnserve.c

Propchange: subversion/branches/1.8.x/
------------------------------------------------------------------------------
  Merged /subversion/trunk:r1481010

Modified: subversion/branches/1.8.x/STATUS
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.8.x/STATUS?rev=1482133&r1=1482132&r2=1482133&view=diff
==============================================================================
--- subversion/branches/1.8.x/STATUS (original)
+++ subversion/branches/1.8.x/STATUS Mon May 13 22:32:01 2013
@@ -139,13 +139,6 @@ Approved changes:
      +1: julianfoad, rhuijben
      +1: stsp (for 1.8.0, but not any 1.8.x since it changes the UI)
 
- * r1481010
-   Fix issue 4365, multi-threaded svnserve pool memory use after clear.
-   Justification:
-     SEGV, particularly on Windows.
-   Votes:
-     +1: philip, danielsh, ivan
-
  * r1481800, r1481813
    Use result_pool for items copied to output array.
    Justification:

Modified: subversion/branches/1.8.x/subversion/svnserve/svnserve.c
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/svnserve/svnserve.c?rev=1482133&r1=1482132&r2=1482133&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/svnserve/svnserve.c (original)
+++ subversion/branches/1.8.x/subversion/svnserve/svnserve.c Mon May 13 
22:32:01 2013
@@ -53,6 +53,7 @@
 
 #include "private/svn_dep_compat.h"
 #include "private/svn_cmdline_private.h"
+#include "private/svn_atomic.h"
 
 #include "winservice.h"
 
@@ -378,11 +379,44 @@ static apr_status_t redirect_stdout(void
   return apr_file_dup2(out_file, err_file, pool);
 }
 
+#if APR_HAS_THREADS
+/* The pool passed to apr_thread_create can only be released when both
+
+      A: the call to apr_thread_create has returned to the calling thread
+      B: the new thread has started running and reached apr_thread_start_t
+
+   So we set the atomic counter to 2 then both the calling thread and
+   the new thread decrease it and when it reaches 0 the pool can be
+   released.  */
+struct shared_pool_t {
+  svn_atomic_t count;
+  apr_pool_t *pool;
+};
+
+static struct shared_pool_t *
+attach_shared_pool(apr_pool_t *pool)
+{
+  struct shared_pool_t *shared = apr_palloc(pool, sizeof(struct 
shared_pool_t));
+
+  shared->pool = pool;
+  svn_atomic_set(&shared->count, 2);
+
+  return shared;
+}
+
+static void
+release_shared_pool(struct shared_pool_t *shared)
+{
+  if (svn_atomic_dec(&shared->count) == 0)
+    svn_pool_destroy(shared->pool);
+}
+#endif
+
 /* "Arguments" passed from the main thread to the connection thread */
 struct serve_thread_t {
   svn_ra_svn_conn_t *conn;
   serve_params_t *params;
-  apr_pool_t *pool;
+  struct shared_pool_t *shared_pool;
 };
 
 #if APR_HAS_THREADS
@@ -390,8 +424,8 @@ static void * APR_THREAD_FUNC serve_thre
 {
   struct serve_thread_t *d = data;
 
-  svn_error_clear(serve(d->conn, d->params, d->pool));
-  svn_pool_destroy(d->pool);
+  svn_error_clear(serve(d->conn, d->params, d->shared_pool->pool));
+  release_shared_pool(d->shared_pool);
 
   return NULL;
 }
@@ -455,6 +489,7 @@ int main(int argc, const char *argv[])
 #if APR_HAS_THREADS
   apr_threadattr_t *tattr;
   apr_thread_t *tid;
+  struct shared_pool_t *shared_pool;
 
   struct serve_thread_t *thread_data;
 #endif
@@ -1093,6 +1128,7 @@ int main(int argc, const char *argv[])
              particularly sophisticated strategy for a threaded server, it's
              little different from forking one process per connection. */
 #if APR_HAS_THREADS
+          shared_pool = attach_shared_pool(connection_pool);
           status = apr_threadattr_create(&tattr, connection_pool);
           if (status)
             {
@@ -1112,9 +1148,9 @@ int main(int argc, const char *argv[])
           thread_data = apr_palloc(connection_pool, sizeof(*thread_data));
           thread_data->conn = conn;
           thread_data->params = &params;
-          thread_data->pool = connection_pool;
+          thread_data->shared_pool = shared_pool;
           status = apr_thread_create(&tid, tattr, serve_thread, thread_data,
-                                     connection_pool);
+                                     shared_pool->pool);
           if (status)
             {
               err = svn_error_wrap_apr(status, _("Can't create thread"));
@@ -1122,6 +1158,7 @@ int main(int argc, const char *argv[])
               svn_error_clear(err);
               exit(1);
             }
+          release_shared_pool(shared_pool);
 #endif
           break;