Re: Thread issues in svnserve

Philip Martin <philip.martin_at_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