Bug 2571 - windows.rename has a needless temp name race
Summary: windows.rename has a needless temp name race
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: unspecified
Hardware: All All
: normal bug
Assignee: Adrian Buehlmann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-29 06:40 UTC by Adrian Buehlmann
Modified: 2010-12-31 13:02 UTC (History)
1 user (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Buehlmann 2010-12-29 06:40 UTC
(creating a bugreport in a obvious attempt to get my patch into stable)

In windows.py, function rename (line 288), we currently use os.path.exists
to check if a temp file name is available.

Trusting os.path.exists is harmful, since using it enables the
following racy sequence of actions:

 1) os.path.exists(temp) returns False
 2) some evil other process creates a file with name temp
 3) os.rename(dst, temp) now fails because temp has been taken

On Windows, os.rename reliably raises OSError with errno.EEXIST if the
destination already exists (even on shares served by Samba).

Windows does *not* silently overwrite the destination of a rename.

So there is no need to first call os.path.exists on the chosen temp path.

The comment in windows.py:

        # The usual race condition this introduces can't be avoided as
        # we need the name to rename into, and not the file itself. Due
        # to the nature of the operation however, any races will at worst
        # lead to the rename failing and the current operation aborting.

is thus simply wrong, since we *can* avoid this race.

BTW, this thing could be another nice way to fail breaking up hardlinks *if*
the resulting rare exception (as claimed by the quoted comment) is swallowed.
Comment 1 Adrian Buehlmann 2010-12-31 13:02 UTC
Fixed with 650314ed845d.
Comment 2 Bugzilla 2012-05-12 09:15 UTC

--- Bug imported by bugzilla@serpentine.com 2012-05-12 09:15 EDT  ---

This bug was previously known as _bug_ 2571 at http://mercurial.selenic.com/bts/issue2571