On 22 July 2014 01:48, <stef...@apache.org> wrote: > Author: stefan2 > Date: Mon Jul 21 21:48:35 2014 > New Revision: 1612405 > > URL: http://svn.apache.org/r1612405 > Log: > Follow-up to r1611379: Correctly handle all intermittent errors. > > Since the same code sequence has been used for FSFS since 2007, > factor it out into some private API function. > > * subversion/include/private/svn_io_private.h > (svn_io__file_lock_autocreate): Declare new private API. > > * subversion/libsvn_subr/io.c > (svn_io__file_lock_autocreate): Move the file handling code from > the lock() function here and handle > all ordinary and racy errors. > > * subversion/libsvn_subr/named_atomic.c > (lock): Call the new function and ensure correct thread mutex use. > > Found by: brane > [...]
> > Modified: subversion/trunk/subversion/libsvn_subr/named_atomic.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/named_atomic.c?rev=1612405&r1=1612404&r2=1612405&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_subr/named_atomic.c (original) > +++ subversion/trunk/subversion/libsvn_subr/named_atomic.c Mon Jul 21 > 21:48:35 2014 > @@ -32,6 +32,7 @@ > #include "svn_pools.h" > #include "svn_dirent_uri.h" > #include "svn_io.h" > +#include "private/svn_io_private.h" > > /* Implementation aspects. > * > @@ -274,28 +275,10 @@ init_thread_mutex(void *baton, apr_pool_ > static svn_error_t * > lock(struct mutex_t *mutex) > { > - svn_error_t *err; > - > - /* Intra-process lock */ > - SVN_ERR(svn_mutex__lock(thread_mutex)); > - > - /* Inter-process lock. */ > - err = svn_io_file_lock2(mutex->lock_name, TRUE, FALSE, mutex->pool); > - if (err && APR_STATUS_IS_ENOENT(err->apr_err)) > - { > - /* No lock file? No big deal; these are just empty files anyway. > - Create it and try again. */ > - svn_error_clear(err); > - err = NULL; > - > - SVN_ERR(svn_io_file_create_empty(mutex->lock_name, mutex->pool)); > - SVN_ERR(svn_io_file_lock2(mutex->lock_name, TRUE, FALSE, mutex->pool)); > - } > - > - /* Don't leave us in a semi-locked state ... */ > - return err > - ? svn_mutex__unlock(thread_mutex, err) > - : err; > + SVN_MUTEX__WITH_LOCK(thread_mutex, > + svn_io__file_lock_autocreate(mutex->lock_name, > + mutex->pool)); > + return SVN_NO_ERROR; > } > Stefan, It seems you broke the code (again): SVN_MUTEX__WITH_LOCK() will release THREAD_MUTEX immediately after file lock will be obtained. That means that access to shared memory will not be synchronized on posix platforms (!). And this also could cause undefined behavior when unlock() function will try to unlock THREAD_MUTEX without actually owning it [1]. [1] http://linux.die.net/man/3/pthread_mutex_unlock -- Ivan Zhakov CTO | VisualSVN | http://www.visualsvn.com
