[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: A proposed solution for svn admin directory names

From: Jonathan Malek <jonathan.malek_at_gmail.com>
Date: 2005-06-30 01:34:20 CEST

I wanted to thank everyone for the feedback on the suggestion. This
is my understanding of the chain so far:

1) The getenv() call reduces performance by 1% in the hyper-often call
scenario (the first suggestion)
2) A static with some kind of synchronization, using Joseph's
suggestion would reduce this performance penalty

As to (2), we could use pthread (used by apr) and something like:

pthread_mutex_t svn_admin_dir_mutex = PTHREAD_MUTEX_INITIALIZER ;

char * getadmdir()
{
        static char * szAdmDir = 0 ;

        pthread_mutex_lock( &svn_admin_dir_mutex ) ;
        if( szAdmDir == 0 )
        {
                szAdmDir = getenv( "SVN_ADM_DIR" ) ;
                if( szAdmDir == 0 )
                        szAdmDir = ".svn" ;
        }
        pthread_mutex_unlock( &svn_admin_dir_mutex ) ;

        return szAdmDir ;
}

Thanks,
Jonathan

On 6/29/05, Joseph Galbraith <galb@vandyke.com> wrote:
> Duane Griffin wrote:
> > On Wed, 2005-06-29 at 18:26, Joseph Galbraith wrote:
> >
> >>char* getadmdir()
> >>{
> >> static char* szAdmDir = 0;
> >> if ( szAdmDir == 0 )
> >> {
> >> szAdmDir = ::getenv("SVN_ADM_DIR");
> >> if ( szAdmDir == 0 )
> >> szAdmDir = ".svn";
> >> }
> >>
> >> return szAdmDir;
> >>}
> >
> >
> > Aside from the possibility of non-atomic pointer assignment, there is
> > another failure case where the function returns NULL:
> >
> > Consider the situation with two threads A and B where SVN_ADM_DIR is not
> > set. A and B both enter the function and the first if block. Thread A
> > continues into the second if block and does the assignment. Now thread B
> > executes the getenv line. Thread A executes the return statement,
> > returning the current global value of szAdmDir, which has been set by
> > thread B to NULL. Bang.
> >
> > It really isn't possible to do this sort of thing in a thread-safe
> > manner without synchronization.
>
> Well, given that the non-atomic pointer assignment does seem to kill
> it in the portable sense, it probably isn't worthwhile continuing this
> discussion too much further. (And the objection to the use of
> environment variables for config in general-- though there is precednet
> in the SVN_SSH environment variable.)
>
> But, wouldn't the introduction of a temporary resolve the
> problem you point out above?
>
> char* szEnv = ::getenv("SVN_ADM_DIR");
> if ( szEnv == 0 )
> szAdmDir = ".svn"
> else
> szAdmDir = szEnv;
>
> Now szAdmDir is written exactly one time (in each thread), always
> with a valid value.
>
> Thanks for the careful analysis, by the way. I'll keep an eye
> out for that particular mistaken pattern in my own code.
>
> Thanks,
>
> Joseph
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 30 01:36:03 2005

This is an archived mail posted to the Subversion Dev mailing list.