On 22 July 2014 01:48, <stefan2_at_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
Received on 2014-08-01 19:38:29 CEST