Re: Thread issues in svnserve

[Philip Martin]

Ivan Zhakov <i...@visualsvn.com> writes:

> But currently APR doesn't access apr_threadattr_t from worker thread
> and doesn't need access to apr_thread_t from main thread. So using
> iterpool for apr_threadattr_t and connection_pool for apr_thread_t
> fixes problem, but only with current APR implementation.

I don't think it works.  There are two problems.  The first problem:

        connection_pool = svn_pool_create(...)
        apr_thread_create(... connection_pool)

Here the worker thread destroys connection_pool when the thread
completes.  This can happen before apr_thread_create returns in which
case memory allocated inside apr_thread_create becomes invalid.  This is
the problem you observed.

The second problem:

        connection_pool = svn_pool_create(...)
        subpool = svn_pool_create(...)
        apr_thread_create(... subpool)
        svn_pool_destroy(subpool)

Here the subpool can get destroyed before the thread starts.  That means
that memory allocated inside apr_thread_create becomes invalid before
the thread starts.  In that case the access to the memory in
dummy_worker is invalid.

I think we may be able to create non-detached threads and then
explicitly detach:

Index: subversion/svnserve/svnserve.c
===================================================================
--- subversion/svnserve/svnserve.c      (revision 1480630)
+++ 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).
@@ -1101,14 +1103,6 @@ int main(int argc, const char *argv[])
               svn_error_clear(err);
               exit(1);
             }
-          status = apr_threadattr_detach_set(tattr, 1);
-          if (status)
-            {
-              err = svn_error_wrap_apr(status, _("Can't set detached state"));
-              svn_handle_error2(err, stderr, FALSE, "svnserve: ");
-              svn_error_clear(err);
-              exit(1);
-            }
           thread_data = apr_palloc(connection_pool, sizeof(*thread_data));
           thread_data->conn = conn;
           thread_data->params = &params;
@@ -1122,6 +1116,15 @@ int main(int argc, const char *argv[])
               svn_error_clear(err);
               exit(1);
             }
+          status = apr_thread_detach(tid);
+          if (status)
+            {
+              err = svn_error_wrap_apr(status, _("Can't detach thread"));
+              svn_handle_error2(err, stderr, FALSE, "svnserve: ");
+              svn_error_clear(err);
+              exit(1);
+            }
+          svn_pool_clear(iterpool);
 #endif
           break;
 

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