[PATCH] windows.rename: eliminate temp name race
Adrian Buehlmann
adrian at cadifra.com
Sun Dec 26 17:44:48 CST 2010
# HG changeset patch
# User Adrian Buehlmann <adrian at cadifra.com>
# Date 1293405868 -3600
# Node ID e9652394c81456ea84f86dec93ecebf0c0c1253a
# Parent 1aea66b71f4f915d482ea2be77e5322acf14db19
windows.rename: eliminate temp name race
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 --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -299,20 +299,18 @@ def rename(src, dst):
# 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
+ continue
raise IOError, (errno.EEXIST, "No usable temporary filename found")
- temp = tempname(dst)
- os.rename(dst, temp)
try:
os.unlink(temp)
except:
More information about the Mercurial-devel
mailing list