Have importlib use os.replace() for atomic renaming.

Closes issue #13961. Thanks to Charles-François Natali for the patch.

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 39cf76a3106c3f67b64f2e6bcbd38600d8ec0a79..56afe574827affc1d111fe3e893809b36853c024 100644 (file)
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -137,26 +137,16 @@ def _path_absolute(path):
 
 
 def _write_atomic(path, data):
-    """Best-effort function to write data to a path atomically.
-    Be prepared to handle a FileExistsError if concurrent writing of the
-    temporary file is attempted."""
-    # Renaming should be atomic on most platforms (including Windows).
-    # Under Windows, the limitation is that we can't rename() to an existing
-    # path, while POSIX will overwrite it. But here we don't really care
-    # if there is a glimpse of time during which the final pyc file doesn't
-    # exist.
+    """Function to write data to a path atomically."""
     # id() is used to generate a pseudo-random filename.
     path_tmp = '{}.{}'.format(path, id(path))
     fd = _os.open(path_tmp, _os.O_EXCL | _os.O_CREAT | _os.O_WRONLY, 0o666)
     try:
+        # We first write data to a temporary file, and then use os.replace() to
+        # perform an atomic rename.
         with _io.FileIO(fd, 'wb') as file:
             file.write(data)
-        try:
-            _os.rename(path_tmp, path)
-        except FileExistsError:
-            # Windows (if we had access to MoveFileEx, we could overwrite)
-            _os.unlink(path)
-            _os.rename(path_tmp, path)
+        _os.replace(path_tmp, path)
     except OSError:
         try:
             _os.unlink(path_tmp)
@@ -602,9 +592,8 @@ class _SourceFileLoader(_FileLoader, SourceLoader):
                 return
         try:
             _write_atomic(path, data)
-        except (PermissionError, FileExistsError):
-            # Don't worry if you can't write bytecode or someone is writing
-            # it at the same time.
+        except PermissionError:
+            # Don't worry if you can't write bytecode.
             pass
 
 

diff --git a/Misc/NEWS b/Misc/NEWS
index b2cf410d22c02394ba5fd69ff0c11282ff8d98c5..07ecfc1b447173cc938c7042b919871d14109f18 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -466,6 +466,8 @@ Core and Builtins
 Library
 -------
 
+- Issue #13961: Move importlib over to using os.replace() for atomic renaming.
+
 - Do away with ambiguous level values (as suggested by PEP 328) in
   importlib.__import__() by raising ValueError when level < 0.