[issue2571] windows.rename has a needless temp name race

[Adrian Buehlmann]

New submission from Adrian Buehlmann <adrian at cadifra.com>:

(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.

----------
assignedto: abuehl
messages: 14778
nosy: abuehl
priority: bug
status: in-progress
title: windows.rename has a needless temp name race
topic: windows

____________________________________________________
Mercurial issue tracker <bugs at mercurial.selenic.com>
<http://mercurial.selenic.com/bts/issue2571>
____________________________________________________