> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: donderdag 12 februari 2015 16:59
> To: Subversion Development
> Cc: Philip Martin
> Subject: Re: svn commit: r1659013 -
> /subversion/trunk/subversion/libsvn_subr/dso.c
>
> On 11 February 2015 at 20:48, <philip_at_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?re
> v=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.
+1
Asked the same question yesterday (on irc).
> 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()?
+1
Good suggestions
Bert
Received on 2015-02-12 18:33:50 CET