Re: Thread issues in svnserve

Ivan Zhakov <ivan_at_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