WIN32_RETRY_LOOP and opening read-only files for writing.

[Lieven Govaerts <sv...@mobsol.be>]

Hi,


I maintain some merge scripts written in Java that work with both
SVNKit and JavaHL. During unit testing on Windows XP we noticed that
some simple svn actions, like checkout take an extraordinary amount of
time. E.g. 'svn checkout' over ra_local of a branch with 3 folders and
6 100 bytes files takes appr. 20 seconds.

After some debugging we found that svn spends almost 15 seconds trying
to change a simple username file in ~\Subversion\auth\svn.username
(see attached extract of the Process Monitor log). It turns out this
file already existed and was set read-only. When JavaHL/svn creates
this file it makes it writeable, but SVNKit seems to create it as
read-only - which I suspect is a bug in SVNKit.

Looking at the Process Monitor output I suppose these 15 seconds are
spent in WIN32_RETRY_LOOP, defined in libsvn_subr/io.c.

WIN32_RETRY_LOOP tries in this case to open the file for writing, and
gets an ERROR_ACCESS_DENIED error. Now I know this error can mean lots
of things, so I don't propose to drop it altogether. But can't we add
a check that, if svn gets this error during file opening, it checks
first if the file is read-only before retrying 100 times?

I see we already do something similar in svn_io_remove_file.

Remarks?

Lieven


-----------------------------------------------------------------
14:58:58,4206273	javaw.exe	5976	CreateFile	C:\Documents and
Settings\govaerl\Application
Data\Subversion\auth\svn.username\bfb50728e2262148ad4928381c7940bc	ACCESS
DENIED
Desired Access: Generic Write, Read Attributes, Disposition:
OverwriteIf, Options: Synchronous IO Non-Alert, Non-Directory File,
Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: 0
14:58:58,4207628	javaw.exe	5976	CreateFile	C:\Documents and
Settings\govaerl\Application Data\Subversion\auth\svn.username	SUCCESS
Desired Access: Synchronize, Disposition: Open, Options: Directory,
Synchronous IO Non-Alert, Open For Backup, Attributes: N, ShareMode:
Read, Write, AllocationSize: n/a, OpenResult: Opened
14:58:58,4212682	javaw.exe	5976	CloseFile	C:\Documents and
Settings\govaerl\Application Data\Subversion\auth\svn.username	SUCCESS

14:58:58,4268767	javaw.exe	5976	CreateFile	C:\Documents and
Settings\govaerl\Application
Data\Subversion\auth\svn.username\bfb50728e2262148ad4928381c7940bc	ACCESS
DENIED	Desired Access: Generic Write, Read Attributes, Disposition:
OverwriteIf, Options: Synchronous IO Non-Alert, Non-Directory File,
Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: 0
14:58:58,4270108	javaw.exe	5976	CreateFile	C:\Documents and
Settings\govaerl\Application
Data\Subversion\auth\svn.username	SUCCESS	Desired Access: Synchronize,
Disposition: Open, Options: Directory, Synchronous IO Non-Alert, Open
For Backup, Attributes: N, ShareMode: Read, Write, AllocationSize:
n/a, OpenResult: Opened
14:58:58,4271387	javaw.exe	5976	CloseFile	C:\Documents and
Settings\govaerl\Application Data\Subversion\auth\svn.username	SUCCESS
...

Re: WIN32_RETRY_LOOP and opening read-only files for writing.

["C. Michael Pilato" <cm...@collab.net>]

Lieven Govaerts wrote:
> WIN32_RETRY_LOOP tries in this case to open the file for writing, and
> gets an ERROR_ACCESS_DENIED error. Now I know this error can mean lots
> of things, so I don't propose to drop it altogether. But can't we add
> a check that, if svn gets this error during file opening, it checks
> first if the file is read-only before retrying 100 times?
> 
> I see we already do something similar in svn_io_remove_file.
> 
> Remarks?

Makes sense to me.  We're assuming, of course, that any time we find such a
file read-only that it isn't merely set that way temporarily by some other
process, but I think that's a fair assumption to make.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

RE: WIN32_RETRY_LOOP and opening read-only files for writing.

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: lieven.govaerts@gmail.com [mailto:lieven.govaerts@gmail.com] On
> Behalf Of Lieven Govaerts
> Sent: maandag 1 maart 2010 16:29
> To: Subversion Development
> Subject: WIN32_RETRY_LOOP and opening read-only files for writing.
> 
> Hi,
> 
> 
> I maintain some merge scripts written in Java that work with both
> SVNKit and JavaHL. During unit testing on Windows XP we noticed that
> some simple svn actions, like checkout take an extraordinary amount of
> time. E.g. 'svn checkout' over ra_local of a branch with 3 folders and
> 6 100 bytes files takes appr. 20 seconds.
> 
> After some debugging we found that svn spends almost 15 seconds trying
> to change a simple username file in ~\Subversion\auth\svn.username
> (see attached extract of the Process Monitor log). It turns out this
> file already existed and was set read-only. When JavaHL/svn creates
> this file it makes it writeable, but SVNKit seems to create it as
> read-only - which I suspect is a bug in SVNKit.
> 
> Looking at the Process Monitor output I suppose these 15 seconds are
> spent in WIN32_RETRY_LOOP, defined in libsvn_subr/io.c.
> 
> WIN32_RETRY_LOOP tries in this case to open the file for writing, and
> gets an ERROR_ACCESS_DENIED error. Now I know this error can mean lots
> of things, so I don't propose to drop it altogether. But can't we add
> a check that, if svn gets this error during file opening, it checks
> first if the file is read-only before retrying 100 times?
> 
> I see we already do something similar in svn_io_remove_file.
> 
> Remarks?

+1 on adding that check before the first retry, like we do in a few other
places. 

(But please don't do some testing before the initial operation, as we do in
at least one place. This fallback should never be necessary on files we just
handle ourselves).

Thanks,
	Bert