[PATCH 2 of 2] Reorder rename operations to minimise risk of leaving repository in unknown state

[Laurens Holst]

Op 29-9-2009 3:54, Steve Borho schreef:
>> How so? I don’t really follow why it wouldn’t…
>>
>> Let’s call the original file A and the new file B:
>>
>> 1. os.rename A to temp (succeeds)
>> 2. AVG starts scanning and gets a lock on A
>>      
> AVG scans the new file (temp).
>
>    
>> 3. os.rename B to dst (succeeds)
>>      
> Still fails (see below).
>
>    
>> 4. os.unlink A (fails because of lock, but the new dirstate is in place)
>>      
> I'm hoping Sune will jump in here and explain this better.  But it's
> my understanding that reordering the unlink() and rename() will not
> resolve the problem like I expected.  The second rename will not
> succeed even if the original (A) file was renamed, if the renamed
> (temp) file is held open and locked.  Something about the way Windows
> locks with directory entries instead of the physical files.
>    

I don’t think that’s true. That would only be the case if AVG already 
locked the file before step 1, but that is not the case. We established 
this in the discussion in TortoiseHg issue 580 [1]. The thing here is 
that the ‘remove’ and ‘rename’ sharing permissions are the same thing 
[2]. Thus, if you cannot remove a file, you cannot rename it either, 
hence AVG must be opening the file *after* it has been renamed.

So yes, Windows temporarily holds directory entries, but *only* if the 
FILE_SHARE_DELETE flag has been set (by AVG), which it has not because 
otherwise there would not be a problem in the first place, or at least 
not on the unlink line. If the FILE_SHARE_DELETE flag has not been set, 
it will simply deny the rename/delete and throw an exception or some such.

By the way, following this line of thought, if some scanner would open 
the file before it is renamed, even if it is a good citizen and has the 
FILE_SHARE_DELETE flag set, there would be no way to prevent problems. 
Unless, of course, a rename does not actually ‘hold’ the old directory 
entry like delete does (which would make sense, as the technical reason 
to do this — NTFS having no inodes and thus requiring a directory entry 
to avoid data loss on power failure — only applies to delete). But anyway.

So anyway, based on the above, I think this will work :).

~Laurens

[1] 
http://bitbucket.org/tortoisehg/stable/issue/580/exception-after-using-revert-in-tortoisehg
[2] http://msdn.microsoft.com/en-us/library/aa363858%28VS.85%29.aspx

-- 
~~ Ushiko-san! Kimi wa doushite, Ushiko-san nan da!! ~~
Laurens Holst, developer, Utrecht, the Netherlands
Website: www.grauw.nl. Backbase employee; www.backbase.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: laurens_nospam.vcf
Type: text/x-vcard
Size: 107 bytes
Desc: not available
Url : http://selenic.com/pipermail/mercurial-devel/attachments/20090929/662d4ff0/attachment.vcf