Skip to content

Commit

Permalink
Attempt to solve race-condition which corrupts .pyc files on Windows
Browse files Browse the repository at this point in the history
This uses of the `atomicwrites` library.

This is very hard to create a reliable test for.

Fix #3008
  • Loading branch information
nicoddemus committed Apr 12, 2018
1 parent f79b032 commit aa95a42
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 33 deletions.
41 changes: 14 additions & 27 deletions _pytest/assertion/rewrite.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
import sys
import types

import atomicwrites
import py

from _pytest.assertion import util


Expand Down Expand Up @@ -140,7 +142,7 @@ def find_module(self, name, path=None):
# 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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions changelog/3008.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A rare race-condition which might result in corrupted ``.pyc`` files on Windows has been hopefully solved.
1 change: 1 addition & 0 deletions changelog/3008.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``pytest`` now depends on the `python-atomicwrites <https://github.com/untitaker/python-atomicwrites>`_ library.
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions testing/test_assertrewrite.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,22 +839,22 @@ def test_meta_path():
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):
Expand Down

0 comments on commit aa95a42

Please sign in to comment.