Re: Thread issues in svnserve

[Philip Martin]

Philip Martin <philip.mar...@wandisco.com> writes:

> We could do this:
>
> Index: subversion/svnserve/svnserve.c
> ===================================================================
> --- subversion/svnserve/svnserve.c    (revision 1480565)
> +++ subversion/svnserve/svnserve.c    (working copy)
> @@ -444,6 +444,7 @@ int main(int argc, const char *argv[])
>    apr_sockaddr_t *sa;
>    apr_pool_t *pool;
>    apr_pool_t *connection_pool;
> +  apr_pool_t *iterpool;
>    svn_error_t *err;
>    apr_getopt_t *os;
>    int opt;
> @@ -973,6 +974,7 @@ int main(int argc, const char *argv[])
>        {
>  #if APR_HAS_THREADS
>          settings.single_threaded = FALSE;
> +        iterpool = svn_pool_create(pool);
>  #else
>          /* No requests will be processed at all
>           * (see "switch (handling_mode)" code further down).
> @@ -1093,7 +1095,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
> -          status = apr_threadattr_create(&tattr, connection_pool);
> +          status = apr_threadattr_create(&tattr, iterpool);
>            if (status)
>              {
>                err = svn_error_wrap_apr(status, _("Can't create threadattr"));
> @@ -1122,6 +1124,7 @@ int main(int argc, const char *argv[])
>                svn_error_clear(err);
>                exit(1);
>              }
> +          svn_pool_clear(iterpool);
>  #endif
>            break;

It might be better to pass iterpool instead of connection_pool into
apr_thread_create.  The problem with tattr also applies to memory
allocated by apr_thread_create such as *new.  At present *new is not
accessed after the thread has started but the implementation could
change.  So new patch:

Index: subversion/svnserve/svnserve.c
===================================================================
--- subversion/svnserve/svnserve.c      (revision 1480608)
+++ subversion/svnserve/svnserve.c      (working copy)
@@ -455,6 +455,7 @@ int main(int argc, const char *argv[])
 #if APR_HAS_THREADS
   apr_threadattr_t *tattr;
   apr_thread_t *tid;
+  apr_pool_t *iterpool;
 
   struct serve_thread_t *thread_data;
 #endif
@@ -973,6 +974,7 @@ int main(int argc, const char *argv[])
       {
 #if APR_HAS_THREADS
         settings.single_threaded = FALSE;
+        iterpool = svn_pool_create(pool);
 #else
         /* No requests will be processed at all
          * (see "switch (handling_mode)" code further down).
@@ -1093,7 +1095,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
-          status = apr_threadattr_create(&tattr, connection_pool);
+          status = apr_threadattr_create(&tattr, iterpool);
           if (status)
             {
               err = svn_error_wrap_apr(status, _("Can't create threadattr"));
@@ -1114,7 +1116,7 @@ int main(int argc, const char *argv[])
           thread_data->params = &params;
           thread_data->pool = connection_pool;
           status = apr_thread_create(&tid, tattr, serve_thread, thread_data,
-                                     connection_pool);
+                                     iterpool);
           if (status)
             {
               err = svn_error_wrap_apr(status, _("Can't create thread"));
@@ -1122,6 +1124,7 @@ int main(int argc, const char *argv[])
               svn_error_clear(err);
               exit(1);
             }
+          svn_pool_clear(iterpool);
 #endif
           break;
 

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download