# HG changeset patch # User Adrian Buehlmann # Date 1293408751 -3600 # Node ID 650314ed845d9fb1940d93beb465ddd738f252ff # Parent 5b83ab614dabd1f1a6eebe77dee14594e4932c15 windows.rename: eliminate temp name race (issue2571) 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. Trusting os.path.exists is actually 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 Not using os.path.exists and directly trying os.rename(dst, temp) eliminates this race. diff -r 5b83ab614dab -r 650314ed845d mercurial/windows.py --- a/mercurial/windows.py Mon Dec 13 22:38:06 2010 +0100 +++ b/mercurial/windows.py Mon Dec 27 01:12:31 2010 +0100 @@ -299,20 +299,18 @@ # rename is safe to do. # The temporary name is chosen at random to avoid the situation # where a file is left lying around from a previous aborted run. - # 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. - def tempname(prefix): - for tries in xrange(10): - temp = '%s-%08x' % (prefix, random.randint(0, 0xffffffff)) - if not os.path.exists(temp): - return temp + for tries in xrange(10): + temp = '%s-%08x' % (dst, random.randint(0, 0xffffffff)) + try: + os.rename(dst, temp) # raises OSError EEXIST if temp exists + break + except OSError, e: + if e.errno != errno.EEXIST: + raise + else: raise IOError, (errno.EEXIST, "No usable temporary filename found") - temp = tempname(dst) - os.rename(dst, temp) try: os.unlink(temp) except: