The bug described below is a race condition in the httpv1 protocol.
Since it doesn't affect httpv2, and mainly affects httpv1 in
high-concurrency situations (which would probably use httpv2 rather than
httpv1), I'm not sure we have any practical need to fix it. However,
I'll document it here for posterity.
---
This was found by Philip's pre-release test runs.
Concurrent httpv1 commits sometimes trigger an SVN_ERR_FS_CONFLICT error
although they should merge() correctly. The cause appears to be a race
condition in the httpv1 client code.
In subversion/libsvn_ra_serf/commit.c:open_root(), if the HEAD revision changes
between the MKACTIVITY request (line 1348) and the subsequent PROPFIND
/!svn/vcc/default request (svn_ra_serf__discover_vcc(), line 1354) will return
a VCC different from the txn's base revision, which leads dav_svn__checkout()
to fail the request at:
return dav_svn__new_error_svn
(resource->pool, HTTP_CONFLICT, SVN_ERR_FS_CONFLICT, 0,
"version resource newer than txn (restart the commit)");
This only affects servers configured with "SVNAdvertiseV2Protocol off", but is
reproducible with trunk:
[The patch triggers the race by forcing a specific linearization of two
concurrent commit operations]
% svn patch mergeinfo_race-stall-MKACTIVITY.diff
% touch /tmp/svn.choose
% make -s svn
% APACHE_MPM=event USE_HTTPV1=yes ./davautocheck.sh svnadmin mergeinfo_race
W: subversion/svn/commit-cmd.c:185,
W: subversion/libsvn_client/commit.c:992,
W: subversion/libsvn_client/commit.c:156: (apr_err=SVN_ERR_WC_NOT_UP_TO_DATE)
W: svn: E155011: Commit failed (details follow):
W: subversion/libsvn_client/commit.c:904,
W: subversion/libsvn_client/commit_util.c:1884,
W: subversion/libsvn_delta/path_driver.c:264,
W: subversion/libsvn_client/commit_util.c:1834,
W: subversion/libsvn_client/commit_util.c:94:
(apr_err=SVN_ERR_WC_NOT_UP_TO_DATE)
W: svn: E155011: Directory
'/home/daniel/src/svn/t1/subversion/tests/cmdline/svn-test-work/working_copies/svnadmin_tests-30.2/d2'
is out of date
W: subversion/libsvn_ra_serf/commit.c:1582,
W: subversion/libsvn_ra_serf/commit.c:406,
W: subversion/libsvn_ra_serf/commit.c:344,
W: subversion/libsvn_ra_serf/commit.c:284,
W: subversion/libsvn_ra_serf/util.c:988,
W: subversion/libsvn_ra_serf/util.c:937,
W: subversion/libsvn_ra_serf/util.c:902,
W: subversion/libsvn_ra_serf/multistatus.c:560: (apr_err=SVN_ERR_FS_CONFLICT)
W: svn: E160024: version resource newer than txn (restart the commit)
Note that retry_checkout_node() warns about — and tries to work around
— a similar race condition.
Cheers,
Daniel
P.S. Sorry about the unix-specific code in the patch; I tried the APR variant
(apr_proc_mutex_lock) first, but it didn't effect mutual exclusion, so rather
than figure out the bug in my code I just resorted to a straight flock().
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 1693669)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -44,6 +44,7 @@
#include "ra_serf.h"
#include "../libsvn_ra/ra_loader.h"
+#include <sys/file.h>
/* Baton passed back with the commit editor. */
typedef struct commit_context_t {
@@ -1240,6 +1241,13 @@ post_response_handler(serf_request_t *request,
+static void choose(apr_pool_t *result_pool)
+{
+ int fd = open("/tmp/svn.choose", O_RDONLY);
+ SVN_ERR_ASSERT_NO_RETURN(fd >= 0);
+ SVN_ERR_ASSERT_NO_RETURN(0 == flock(fd, LOCK_EX));
+}
+
/* Commit baton callbacks */
static svn_error_t *
@@ -1347,6 +1355,14 @@ open_root(void *edit_baton,
SVN_ERR(svn_ra_serf__context_run_one(handler, scratch_pool));
+ /* let both MKACTIVITY finish before either MERGE */
+ sleep(2);
+
+ /* The first process to get here will get the mutex and proceed, MERGE,
+ * and exit. Only at that point will the second to get here proceed.
+ */
+ choose(commit_ctx->pool);
+
if (handler->sline.code != 201)
return svn_error_trace(svn_ra_serf__unexpected_status(handler));
Index: subversion/tests/cmdline/davautocheck.sh
===================================================================
--- subversion/tests/cmdline/davautocheck.sh (revision 1693669)
+++ subversion/tests/cmdline/davautocheck.sh (working copy)
@@ -502,7 +502,7 @@ MaxRequestsPerChild 0
</IfModule>
MaxClients 32
HostNameLookups Off
-LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" format
+LogFormat "%D %l %u %t \"%r\" %>s %b \"%{remote}p\"" format
CustomLog "$HTTPD_ROOT/req" format
CustomLog "$HTTPD_ROOT/ops" "%t %u %{SVN-REPOS-NAME}e %{SVN-ACTION}e" env=SVN-ACTION
Index: subversion/tests/cmdline/svnadmin_tests.py
===================================================================
--- subversion/tests/cmdline/svnadmin_tests.py (revision 1693669)
+++ subversion/tests/cmdline/svnadmin_tests.py (working copy)
@@ -2022,10 +2022,6 @@ def mergeinfo_race(sbox):
svntest.main.run_svn(None, 'mkdir', sbox.ospath('d1', wc_dir))
svntest.main.run_svn(None, 'mkdir', sbox.ospath('d2', wc2_dir))
- ## Set random mergeinfo properties.
- svntest.main.run_svn(None, 'ps', 'svn:mergeinfo', '/P:42', sbox.ospath('A', wc_dir))
- svntest.main.run_svn(None, 'ps', 'svn:mergeinfo', '/Q:42', sbox.ospath('iota', wc2_dir))
-
def makethread(some_wc_dir):
def worker():
svntest.main.run_svn(None, 'commit', '-mm', some_wc_dir)
