Merge pull request #4844 from mwichmann/fix/cachedir-race-again

Further tweak CacheDir for races

Author: bdbaddog

CHANGES.txt

@@ -72,6 +72,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       the general unavailablility of the obsolete build-time Qt3 package.
     - A few more typing cleanups in Environment (and in one case which
       affected it in the Node package).
+    - Adjust race protection for CacheDir - now uses a unique temporary
+      directory for each writer before doing the move, instead of depending
+      on a one-time uuid to make a path to the file.
     - Clarify VariantDir behavior when switching to not duplicate sources
       and tweak wording a bit.
 

RELEASE.txt

@@ -47,6 +47,10 @@ FIXES
 
 - Fix --debug=includes for case of multiple source files.
 
+- Adjust race protection for CacheDir - now uses a unique temporary
+  directory for each writer before doing the move, instead of depending
+  on a one-time uuid to make a path to the file.
+
 IMPROVEMENTS
 ------------
 

SCons/CacheDir.py

@@ -31,10 +31,10 @@
 import stat
 import sys
 import tempfile
-import uuid
 
 import SCons.Action
 import SCons.Errors
+# import SCons.Node.FS  # used for hash_chunksice, but causes import loop
 import SCons.Warnings
 import SCons.Util
 
@@ -49,7 +49,6 @@
 cache_force = False
 cache_show = False
 cache_readonly = False
-cache_tmp_uuid = uuid.uuid4().hex
 
 def CacheRetrieveFunc(target, source, env) -> int:
     t = target[0]
@@ -110,7 +109,6 @@ def CachePushFunc(target, source, env) -> None:
 
     cd.CacheDebug('CachePush(%s):  pushing to %s\n', t, cachefile)
 
-    tempfile = "%s.tmp%s"%(cachefile,cache_tmp_uuid)
     errfmt = "Unable to copy %s to cache. Cache file is %s"
 
     try:
@@ -119,11 +117,13 @@ def CachePushFunc(target, source, env) -> None:
         msg = errfmt % (str(target), cachefile)
         raise SCons.Errors.SConsEnvironmentError(msg)
     try:
-        if fs.islink(t.get_internal_path()):
-            fs.symlink(fs.readlink(t.get_internal_path()), tempfile)
-        else:
-            cd.copy_to_cache(env, t.get_internal_path(), tempfile)
-        fs.rename(tempfile, cachefile)
+        with tempfile.TemporaryDirectory(dir=cachedir) as temp_dir:
+            temp_file = os.path.join(temp_dir, os.path.basename(cachefile))
+            if fs.islink(t.get_internal_path()):
+                fs.symlink(fs.readlink(t.get_internal_path()), temp_file)
+            else:
+                cd.copy_to_cache(env, t.get_internal_path(), temp_file)
+            fs.rename(temp_file, cachefile)
 
     except OSError:
         # It's possible someone else tried writing the file at the