On 11 February 2015 at 20:48, <phi...@apache.org> wrote: > Author: philip > Date: Wed Feb 11 17:48:06 2015 > New Revision: 1659013 > > URL: http://svn.apache.org/r1659013 > Log: > Wrap svn_dso_initialize2 call with svn_atomic__init_once, this > fixes a crash in the DSO hash code when running the C tests in > parallel. > > * subversion/libsvn_subr/dso.c > (atomic_init_func): New. > (svn_dso_load): Use svn_atomic__init_once. > > Modified: > subversion/trunk/subversion/libsvn_subr/dso.c > > Modified: subversion/trunk/subversion/libsvn_subr/dso.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dso.c?rev=1659013&r1=1659012&r2=1659013&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_subr/dso.c (original) > +++ subversion/trunk/subversion/libsvn_subr/dso.c Wed Feb 11 17:48:06 2015 > @@ -28,6 +28,7 @@ > #include "svn_private_config.h" > > #include "private/svn_mutex.h" > +#include "private/svn_atomic.h" > > /* A mutex to protect our global pool and cache. */ > static svn_mutex__t *dso_mutex = NULL; > @@ -41,6 +42,8 @@ static apr_hash_t *dso_cache; > /* Just an arbitrary location in memory... */ > static int not_there_sentinel; > > +static volatile svn_atomic_t atomic_init_status = 0; > + > /* A specific value we store in the dso_cache to indicate that the > library wasn't found. This keeps us from allocating extra memory > from dso_pool when trying to find libraries we already know aren't > @@ -61,6 +64,14 @@ svn_dso_initialize2(void) > return SVN_NO_ERROR; > } > > +static svn_error_t * > +atomic_init_func(void *baton, > + apr_pool_t *pool) > +{ > + SVN_ERR(svn_dso_initialize2()); > + return SVN_NO_ERROR; > +} > + > #if APR_HAS_DSO > static svn_error_t * > svn_dso_load_internal(apr_dso_handle_t **dso, const char *fname) > @@ -107,8 +118,8 @@ svn_dso_load_internal(apr_dso_handle_t * > svn_error_t * > svn_dso_load(apr_dso_handle_t **dso, const char *fname) > { > - if (! dso_pool) > - SVN_ERR(svn_dso_initialize2()); > + SVN_ERR(svn_atomic__init_once(&atomic_init_status, atomic_init_func, > + NULL, NULL)); > Philip,
The fix itself makes sense, but I agree with Evgeny that long-term fix should be add call to svn_dso_initialize2() in the test suite like we do in all application and mod_dav_svn. Also current fix doesn't protect from svn_dso_initialize2() executing concurrently if one thread call svn_dso_initialize2() directly, while other thread will call it indirectly through svn_dso_load(). Why you didn't make svn_atomic__init_once() internal implementation details of svn_dso_initialize2()? -- Ivan Zhakov
