[The Apache mailserver is down; see http://status.apache.org/]
It doesn’t even get into the mutex lock code, because the mutex is NULL on Windows.
So the code that tries to avoid the deadlock doesn’t run and the code deadlocks on the actual file lock.
Bert
From: Stefan Fuhrmann [mailto:stefan.fuhrmann_at_wandisco.com]
Sent: dinsdag 6 mei 2014 00:22
To: Ivan Zhakov
Cc: Bert Huijben; Subversion Development
Subject: Re: svn commit: r1591919 - in /subversion/trunk/subversion: include/ include/private/ libsvn_fs/ libsvn_fs_base/bdb/ libsvn_fs_fs/ libsvn_fs_x/ libsvn_ra_svn/ libsvn_subr/ svnserve/ tests/ tests/libsvn_fs_fs/
On Mon, May 5, 2014 at 2:57 PM, Ivan Zhakov <ivan_at_visualsvn.com <mailto:ivan_at_visualsvn.com> > wrote:
On 5 May 2014 16:47, Bert Huijben <bert_at_qqmail.nl <mailto:bert_at_qqmail.nl> > wrote:
>
>
>> -----Original Message-----
>> From: stefan2_at_apache.org <mailto:stefan2_at_apache.org> [mailto:stefan2_at_apache.org <mailto:stefan2_at_apache.org> ]
>> Sent: vrijdag 2 mei 2014 16:08
>> To: commits_at_subversion.apache.org <mailto:commits_at_subversion.apache.org>
>> Subject: svn commit: r1591919 - in /subversion/trunk/subversion: include/
>> include/private/ libsvn_fs/ libsvn_fs_base/bdb/ libsvn_fs_fs/ libsvn_fs_x/
>> libsvn_ra_svn/ libsvn_subr/ svnserve/ tests/ tests/libsvn_fs_fs/
>>
>> Author: stefan2
>> Date: Fri May 2 14:07:40 2014
>> New Revision: 1591919
>>
>> URL: http://svn.apache.org/r1591919
>> Log:
>> APR mutexes don't support recursive locking on all platforms.
>> As a result, trying to take out the same lock twice in the
>> same thread will cause a lock up under e.g. Linux. This patch
>> adds an option to svn_mutex__t that detects recursive locking
>> attempts in most cases and returns a proper error.
>>
>> The idea is simply to store the thread ID of the lock OWNER
>> along the actual mutex object. If that matches the current
>> thread's ID, there is a violation. As the current thread
>> cannot race with itself and because any other thread uses a
>> different thread ID, setting and comparing this aux. info can
>> be done safely.
>>
>> Also, we may allow for false negatives here since we only try
>> to detect code sequences that are already illegal in the first
>> place. We also don't make that check mandatory as access to
>> thread IDs and their comparison may be somewhat expensive on
>> some systems - which would impair the futex-like behavior that
>> we assume in some places like the caches.
>>
>> A more detailed description has been added to the source code.
>> A FSFS-based test is added as that has been the origin of the
>> feature request.
>>
>> Current users will be updated as follows. FS level locks and
>> library / module initialization will enable recursion detection.
>> Potentially runtime critical, internal use disables it.
>>
>> * subversion/include/svn_error_codes.h
>> (SVN_ERR_RECURSIVE_LOCK): Define a new error code for invalid
>> locking schemes.
>>
>> * subversion/include/private/svn_mutex.h
>> (svn_mutex__t): Our mutex is now a struct as we add aux.
>> data to it.
>> (svn_mutex__init): Add the CHECKED option.
>>
>> * subversion/libsvn_subr/mutex.c
>> (svn_mutex__t): Define the mutex structure and and document
>> the aux. data extensively.
>> (svn_mutex__init): Update constructor.
>> (svn_mutex__lock): Optionally, check for recursive locking attempts
>> and update the associtated aux. data.
>> (svn_mutex__unlock): Optionally, update the aux. data for recursive
>> lock detection.
>>
>> * subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
>> (never_reached,
>> lock_again): Callback functions required by the new test.
>> (recursive_locking): New test expecting an SVN_ERR_RECURSIVE_LOCK.
>> (test_funcs): Register the new test.
>
> This new test currently causes a deadlock on Windows and a test failure on OpenBSD.
>
[...]
> Windows:
> Hang in
> [[[
>> libapr-1.dll!apr_file_lock(apr_file_t * thefile, int type) Line 35 C
> fs-fs-pack-test.exe!svn_io_lock_open_file(apr_file_t * lockfile_handle, int exclusive, int nonblocking, apr_pool_t * pool) Line 2137 C
> fs-fs-pack-test.exe!svn_io_file_lock2(const char * lock_file, int exclusive, int nonblocking, apr_pool_t * pool) Line 2239 C
> fs-fs-pack-test.exe!get_lock_on_filesystem(const char * lock_filename, apr_pool_t * pool) Line 115 C
> fs-fs-pack-test.exe!with_some_lock_file(with_lock_baton_t * baton) Line 202 C
> fs-fs-pack-test.exe!with_lock(void * baton, apr_pool_t * pool) Line 248 C
> fs-fs-pack-test.exe!svn_fs_fs__with_all_locks(svn_fs_t * fs, svn_error_t * (void *, apr_pool_t *) * body, void * baton, apr_pool_t * pool) Line 424 C
> fs-fs-pack-test.exe!lock_again(void * baton, apr_pool_t * pool) Line 1148 C
> fs-fs-pack-test.exe!with_some_lock_file(with_lock_baton_t * baton) Line 230 C
> fs-fs-pack-test.exe!with_lock(void * baton, apr_pool_t * pool) Line 248 C
> fs-fs-pack-test.exe!with_some_lock_file(with_lock_baton_t * baton) Line 230 C
> fs-fs-pack-test.exe!with_lock(void * baton, apr_pool_t * pool) Line 248 C
> fs-fs-pack-test.exe!with_some_lock_file(with_lock_baton_t * baton) Line 230 C
> fs-fs-pack-test.exe!with_lock(void * baton, apr_pool_t * pool) Line 248 C
> fs-fs-pack-test.exe!svn_fs_fs__with_all_locks(svn_fs_t * fs, svn_error_t * (void *, apr_pool_t *) * body, void * baton, apr_pool_t * pool) Line 424 C
> fs-fs-pack-test.exe!recursive_locking(const svn_test_opts_t * opts, apr_pool_t * pool) Line 1159 C
> fs-fs-pack-test.exe!test_thread(apr_thread_t * thread, void * data) Line 519 C
> libapr-1.dll!dummy_worker(void * opaque) Line 79 C
> ]]]
>
Most likely this happens because file locks are *per file handle* on
Windows: not per-process as on posix. That means that one thread
cannot obtain lock to file using different file handles.
Well, it should not get to the point where it would
attempt to take out the second file lock. The recursive
mutex lock should be detected prior to that.
Could you have a lock what thread IDs we get in
svn_mutex__lock? This is strictly single-threaded code
and should be easy to debug. I simply have no windows
machine available ATM.
-- Stefan^2.
Received on 2014-05-15 18:42:06 CEST