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
> Log:
> 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
> (svn_io_make_dir_recursively,
> 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
> directories.
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> 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 */
> + if (apr_err == APR_FROM_OS_ERROR(ERROR_SHARING_VIOLATION))
> + WIN32_RETRY_LOOP(apr_err, apr_dir_make_recursive(path_apr,
> + APR_OS_DEFAULT, pool));
> +#endif

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