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

RE: svn commit: r1617926 -/subversion/trunk/subversion/libsvn_subr/io.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 14 Aug 2014 23:31:43 +0200

I don't think a macro is the right thing to do here. The win32 retry macro already makes it too easy to reuse the existing but bad checks everywhere, causing more unneeded 12 second delays. Only specific errors should be retried in specific cases. Just retrying everything doesn't resolve the real problems. Only on some file lock cases this really makes sense.

Several months ago somebody suggested to just add such a retry around the recursive make directory...
... Great solution to a race condition in the Apr implementation which we eventually found.
(One that applied to multiple platforms if I remember correctly)

I was thinking about completely removing the retry from these mkdir cases, but I'm not completely convinced that there can never be relevant sharing violations.

In the far past many of these loops were just added as quick fixes by non Windows developers to 'fix some user reported Windows problems' which were not completely investigated.


-----Original Message-----
From: "Branko Čibej" <brane_at_wandisco.com>
Sent: ‎14/‎08/‎2014 20:19
To: "dev_at_subversion.apache.org" <dev_at_subversion.apache.org>
Subject: Re: svn commit: r1617926 -/subversion/trunk/subversion/libsvn_subr/io.c

On 14.08.2014 14:55, rhuijben_at_apache.org wrote:

Author: rhuijben
Date: Thu Aug 14 12:55:02 2014
New Revision: 1617926

URL: http://svn.apache.org/r1617926
Resolve a 12 second delay on every 'svn' invocation from Windows system
services that don't have access to a writable user profile.
(Try running a svn script from the task scheduler on Windows 2012 R2)

Without this fix we retry creating the %APPDATA%\Subversion directory again
and again, and then continue silently.

The retry loop prevents erroring out from virusscanners and indexers that keep
file handles open, but this can't happen in the create directory case as we
would have waited in the delete preceding the create in relevant cases.

(Found while rebuilding the Windows buildbot last week)

* subversion/libsvn_subr/io.c
   dir_make): Don't retry for 12 seconds when creating a directory fails,
    because we are using a directory with an ACL that denies creating


Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1617926&r1=1617925&r2=1617926&view=diff
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Thu Aug 14 12:55:02 2014
@@ -1154,8 +1154,13 @@ svn_io_make_dir_recursively(const char *
   SVN_ERR(cstring_from_utf8(&path_apr, path, pool));
   apr_err = apr_dir_make_recursive(path_apr, APR_OS_DEFAULT, pool);
- WIN32_RETRY_LOOP(apr_err, apr_dir_make_recursive(path_apr,
- APR_OS_DEFAULT, pool));
+#ifdef WIN32
+ /* Don't retry on ERROR_ACCESS_DENIED, as that typically signals a
+ permanent error */
+ WIN32_RETRY_LOOP(apr_err, apr_dir_make_recursive(path_apr,
+ APR_OS_DEFAULT, pool));
Bert, would it not make sense to wrap this idiom into a Windows-specific retry macro, if only to avoid writing the same code twice in this file (and avoid the #ifdefs here, too)?

-- Brane

Branko Čibej | Director of Subversion 
WANdisco | Realising the impossibilities of Big Data 
e. brane_at_wandisco.com
Received on 2014-08-14 23:33:10 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.