[PATCH 3 of 3 STABLE] localrepo.wwrite: use unlinkopened instead of os.unlink

[Adrian Buehlmann]

On 2010-12-03 03:56, Greg Ward wrote:
> On Wed, Dec 1, 2010 at 7:53 AM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> # HG changeset patch
>> # User Adrian Buehlmann <adrian at cadifra.com>
>> # Date 1291199708 -3600
>> # Branch stable
>> # Node ID 18d06beed4a794331427e89f3828cb9408012b6a
>> # Parent  52f97fe43a6be964691d78a0147f6650855fc303
>> localrepo.wwrite: use unlinkopened instead of os.unlink
>>
>> Windows delays deleting open files, preventing recreation under
>> the same name until the file is closed.
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -618,7 +618,7 @@ class localrepository(repo.repository):
>>     def wwrite(self, filename, data, flags):
>>         data = self._filter(self._decodefilterpats, filename, data)
>>         try:
>> -            os.unlink(self.wjoin(filename))
>> +            util.unlinkopened(self.wjoin(filename))
> 
> Does this fix a bug? Or is just a "nice to have"?

It does. The original code causes another instance of Issue2524.

BTW, I hope you have seen that util.unlinkopened is an alias for os.unlink
on Linux.

BTW2, Matt wants to have that solved in one place. This will take time,
since we have to review every single os.unlink.



Some further (possibly redundant) explanations:

localrepo.wwrite is one of the few places with the biggest damage
potential regarding Issue2524, since all working dir writes go through
this doing posixfile(f, 'w') later on in the opener. Which Mercurial now
has been doing for a long time already.

This also explains the reports we had about vanished files with Virus
scanners very well. But please note that a virus scanncer is not a
requirement to trigger this kind of file loss. They just increase
the file loss probability to a point where users start to notice it.



The whole thing is racy to reproduce with real uses cases. For white box
testing, it is easy to reproduce on Windows.

On Windows, in one shell start python and open a working dir file
with posixfile:

  $ python
  Python 2.6.5 (r265:79096, Mar 19 2010, 21:48:26) [MSC v.1500 32 bit (Intel)] on win32
  Type "help", "copyright", "credits" or "license" for more information.
  >>> from mercurial.windows import posixfile
  >>> fd = posixfile('a.txt')
  >>>

then in a second cmd.exe shell instance do:

  $ hg version
  Mercurial Distributed SCM (version 1.7.2)
  (see http://mercurial.selenic.com for more information)
  
  [..]

  $ hg parent -q
  17:e86f3b869e53

  $ hg update 16
  abort: C:\Users\adi\hgrepos\tests\c\a.txt: Access is denied

Now, the directory entry for 'a.txt' still shows up:

  $ dir /w
   Volume in drive C has no label.
   Volume Serial Number is F80E-0A52

   Directory of C:\Users\adi\hgrepos\tests\c

  [.]     [..]    [.hg]   a.txt   b.txt
                 2 File(s)             18 bytes
                 3 Dir(s)  473'414'610'944 bytes free

But the file a.txt is now a ghost in "scheduled delete" state [1].

In that state you can't even read the file ("type" is the Windows equivalent
of "cat" for the Linux fraction):

  $ type a.txt
  Access is denied.

Now, let's close the file handle in the first shell:

  >>> fd.close()
  >>>

Another look in the second command shell now shows the file is gone from
the directory:

  $ dir /w
   Volume in drive C has no label.
   Volume Serial Number is F80E-0A52

   Directory of C:\Users\adi\hgrepos\tests\c

  [.]     [..]    [.hg]   b.txt
                 1 File(s)              6 bytes
                 3 Dir(s)  473'414'606'848 bytes free

And status agrees with that:

  $ hg status
  ! a.txt

Boom.

In short: doing a 'hg update' while another process is reading the file
nukes the file on Windows.

[1] http://mercurial.selenic.com/wiki/UnlinkingFilesOnWindows