Merge pull request #3390 from nicoddemus/atomic-pyc-writes

Attempt to solve race-condition which corrupts .pyc files on Windows

_pytest/assertion/rewrite.py

@@ -12,7 +12,9 @@ import struct
 import sys
 import types
 
+import atomicwrites
 import py
+
 from _pytest.assertion import util
 
 
@@ -140,7 +142,7 @@ class AssertionRewritingHook(object):
                 # Probably a SyntaxError in the test.
                 return None
             if write:
-                _make_rewritten_pyc(state, source_stat, pyc, co)
+                _write_pyc(state, co, source_stat, pyc)
         else:
             state.trace("found cached rewritten pyc for %r" % (fn,))
         self.modules[name] = co, pyc
@@ -258,22 +260,21 @@ def _write_pyc(state, co, source_stat, pyc):
     # sometime to be able to use imp.load_compiled to load them. (See
     # the comment in load_module above.)
     try:
-        fp = open(pyc, "wb")
-    except IOError:
-        err = sys.exc_info()[1].errno
-        state.trace("error writing pyc file at %s: errno=%s" % (pyc, err))
+        with atomicwrites.atomic_write(pyc, mode="wb", overwrite=True) as fp:
+            fp.write(imp.get_magic())
+            mtime = int(source_stat.mtime)
+            size = source_stat.size & 0xFFFFFFFF
+            fp.write(struct.pack("<ll", mtime, size))
+            if six.PY2:
+                marshal.dump(co, fp.file)
+            else:
+                marshal.dump(co, fp)
+    except EnvironmentError as e:
+        state.trace("error writing pyc file at %s: errno=%s" % (pyc, e.errno))
         # we ignore any failure to write the cache file
         # there are many reasons, permission-denied, __pycache__ being a
         # file etc.
         return False
-    try:
-        fp.write(imp.get_magic())
-        mtime = int(source_stat.mtime)
-        size = source_stat.size & 0xFFFFFFFF
-        fp.write(struct.pack("<ll", mtime, size))
-        marshal.dump(co, fp)
-    finally:
-        fp.close()
     return True
 
 
@@ -338,20 +339,6 @@ def _rewrite_test(config, fn):
     return stat, co
 
 
-def _make_rewritten_pyc(state, source_stat, pyc, co):
-    """Try to dump rewritten code to *pyc*."""
-    if sys.platform.startswith("win"):
-        # Windows grants exclusive access to open files and doesn't have atomic
-        # rename, so just write into the final file.
-        _write_pyc(state, co, source_stat, pyc)
-    else:
-        # When not on windows, assume rename is atomic. Dump the code object
-        # into a file specific to this process and atomically replace it.
-        proc_pyc = pyc + "." + str(os.getpid())
-        if _write_pyc(state, co, source_stat, proc_pyc):
-            os.rename(proc_pyc, pyc)
-
-
 def _read_pyc(source, pyc, trace=lambda x: None):
     """Possibly read a pytest pyc containing rewritten code.
 

changelog/3008.bugfix.rst

@@ -0,0 +1 @@
+A rare race-condition which might result in corrupted ``.pyc`` files on Windows has been hopefully solved.

changelog/3008.trivial.rst

@@ -0,0 +1 @@
+``pytest`` now depends on the `python-atomicwrites <https://github.com/untitaker/python-atomicwrites>`_ library.

setup.py

@@ -61,6 +61,7 @@ def main():
         'setuptools',
         'attrs>=17.4.0',
         'more-itertools>=4.0.0',
+        'atomicwrites>=1.0',
     ]
     # if _PYTEST_SETUP_SKIP_PLUGGY_DEP is set, skip installing pluggy;
     # used by tox.ini to test with pluggy master

testing/test_assertrewrite.py

@@ -839,22 +839,22 @@ class TestAssertionRewriteHookDetails(object):
     def test_write_pyc(self, testdir, tmpdir, monkeypatch):
         from _pytest.assertion.rewrite import _write_pyc
         from _pytest.assertion import AssertionState
-        try:
-            import __builtin__ as b
-        except ImportError:
-            import builtins as b
+        import atomicwrites
+        from contextlib import contextmanager
         config = testdir.parseconfig([])
         state = AssertionState(config, "rewrite")
         source_path = tmpdir.ensure("source.py")
         pycpath = tmpdir.join("pyc").strpath
         assert _write_pyc(state, [1], source_path.stat(), pycpath)
 
-        def open(*args):
+        @contextmanager
+        def atomic_write_failed(fn, mode='r', overwrite=False):
             e = IOError()
             e.errno = 10
             raise e
+            yield  # noqa
 
-        monkeypatch.setattr(b, "open", open)
+        monkeypatch.setattr(atomicwrites, "atomic_write", atomic_write_failed)
         assert not _write_pyc(state, [1], source_path.stat(), pycpath)
 
     def test_resources_provider_for_loader(self, testdir):