Issue #13392: Writing a pyc file should now be atomic under Windows as well.

3 files changed, 60 insertions, 25 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 209e041b76..359b9e7a90 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -84,24 +84,29 @@ 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."""
-    if not sys.platform.startswith('win'):
-        # On POSIX-like platforms, renaming is atomic. 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)
+    # 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.
+    # 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:
+        with _io.FileIO(fd, 'wb') as file:
+            file.write(data)
         try:
-            with _io.FileIO(fd, 'wb') as file:
-                file.write(data)
             _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)
+    except OSError:
+        try:
+            _os.unlink(path_tmp)
         except OSError:
-            try:
-                _os.unlink(path_tmp)
-            except OSError:
-                pass
-            raise
-    else:
-        with _io.FileIO(path, 'wb') as file:
-            file.write(data)
+            pass
+        raise
 
 
 def _wrap(new, old):
diff --git a/Misc/NEWS b/Misc/NEWS
index 334e78722e..ed713e35b2 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,8 @@ What's New in Python 3.3 Alpha 1?
 Core and Builtins
 -----------------
 
+- Issue #13392: Writing a pyc file should now be atomic under Windows as well.
+
 - Issue #13333: The UTF-7 decoder now accepts lone surrogates (the encoder
   already accepts them).
 
diff --git a/Python/import.c b/Python/import.c
index ae1101e4ec..3c3e504e28 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -1197,6 +1197,8 @@ write_compiled_module(PyCodeObject *co, PyObject *cpathname,
     time_t mtime = srcstat->st_mtime;
 #ifdef MS_WINDOWS   /* since Windows uses different permissions  */
     mode_t mode = srcstat->st_mode & ~S_IEXEC;
+    PyObject *cpathname_tmp;
+    Py_ssize_t cpathname_len;
 #else
     mode_t dirmode = (srcstat->st_mode |
                       S_IXUSR | S_IXGRP | S_IXOTH |
@@ -1255,18 +1257,29 @@ write_compiled_module(PyCodeObject *co, PyObject *cpathname,
     }
     Py_DECREF(dirname);
 
+    /* We first write to a tmp file and then take advantage
+       of atomic renaming (which *should* be true even under Windows). */
 #ifdef MS_WINDOWS
-    (void)DeleteFileW(PyUnicode_AS_UNICODE(cpathname));
-    fd = _wopen(PyUnicode_AS_UNICODE(cpathname),
-                 O_EXCL | O_CREAT | O_WRONLY | O_TRUNC | O_BINARY,
-                 mode);
+    cpathname_len = PyUnicode_GET_LENGTH(cpathname);
+    cpathname_tmp = PyUnicode_New(cpathname_len + 4,
+                                  PyUnicode_MAX_CHAR_VALUE(cpathname));
+    if (cpathname_tmp == NULL) {
+        PyErr_Clear();
+        return;
+    }
+    PyUnicode_WriteChar(cpathname_tmp, cpathname_len + 0, '.');
+    PyUnicode_WriteChar(cpathname_tmp, cpathname_len + 1, 't');
+    PyUnicode_WriteChar(cpathname_tmp, cpathname_len + 2, 'm');
+    PyUnicode_WriteChar(cpathname_tmp, cpathname_len + 3, 'p');
+    (void)DeleteFileW(PyUnicode_AS_UNICODE(cpathname_tmp));
+    fd = _wopen(PyUnicode_AS_UNICODE(cpathname_tmp),
+                O_EXCL | O_CREAT | O_WRONLY | O_BINARY,
+                mode);
     if (0 <= fd)
         fp = fdopen(fd, "wb");
     else
         fp = NULL;
 #else
-    /* Under POSIX, we first write to a tmp file and then take advantage
-       of atomic renaming. */
     cpathbytes = PyUnicode_EncodeFSDefault(cpathname);
     if (cpathbytes == NULL) {
         PyErr_Clear();
@@ -1294,7 +1307,9 @@ write_compiled_module(PyCodeObject *co, PyObject *cpathname,
         if (Py_VerboseFlag)
             PySys_FormatStderr(
                 "# can't create %R\n", cpathname);
-#ifndef MS_WINDOWS
+#ifdef MS_WINDOWS
+        Py_DECREF(cpathname_tmp);
+#else
         Py_DECREF(cpathbytes);
         Py_DECREF(cpathbytes_tmp);
 #endif
@@ -1315,7 +1330,8 @@ write_compiled_module(PyCodeObject *co, PyObject *cpathname,
         /* Don't keep partial file */
         fclose(fp);
 #ifdef MS_WINDOWS
-        (void)DeleteFileW(PyUnicode_AS_UNICODE(cpathname));
+        (void)DeleteFileW(PyUnicode_AS_UNICODE(cpathname_tmp));
+        Py_DECREF(cpathname_tmp);
 #else
         (void) unlink(PyBytes_AS_STRING(cpathbytes_tmp));
         Py_DECREF(cpathbytes);
@@ -1324,8 +1340,20 @@ write_compiled_module(PyCodeObject *co, PyObject *cpathname,
         return;
     }
     fclose(fp);
-    /* Under POSIX, do an atomic rename */
-#ifndef MS_WINDOWS
+    /* Do a (hopefully) atomic rename */
+#ifdef MS_WINDOWS
+    if (!MoveFileExW(PyUnicode_AS_UNICODE(cpathname_tmp),
+                     PyUnicode_AS_UNICODE(cpathname),
+                     MOVEFILE_REPLACE_EXISTING)) {
+        if (Py_VerboseFlag)
+            PySys_FormatStderr("# can't write %R\n", cpathname);
+        /* Don't keep tmp file */
+        (void) DeleteFileW(PyUnicode_AS_UNICODE(cpathname_tmp));
+        Py_DECREF(cpathname_tmp);
+        return;
+    }
+    Py_DECREF(cpathname_tmp);
+#else
     if (rename(PyBytes_AS_STRING(cpathbytes_tmp),
                PyBytes_AS_STRING(cpathbytes))) {
         if (Py_VerboseFlag)