Skip to content

Commit

Permalink
XFAIL TestLocalPath.test_make_numbered_dir_multiprocess_safe
Browse files Browse the repository at this point in the history
The tested py.path.local.make_numbered_dir function is *not*
multiprocess safe, because is uses os.listdir which itself is not.

The os.listdir documentation explicitly states that:

> If a file is removed from or added to the directory during the call
> of this function, whether a name for that file be included is unspecified.

This can lead to a race when:

 1. process A attempts to create directory N
 2. the creation fails, as another process already created it in the meantime
 3. process A calls listdir to determine a more recent maxnum
 4. processes B+ repeatedly create newer directories and they delete directory N
 5. process A doesn't have directory N or any newer directory in listdir result
 6. process A attempts to create directory N again and raises

For details, see #11603 (comment)
and bellow.

Additionally, the test itself has a race in batch_make_numbered_dirs.
When this functions attempts to write to repro-N/foo,
repro-N may have already been removed by another process.

For details, see #11603 (comment)
and bellow.

---

The tested py.path.local.make_numbered_dir function is not used in pytest.
There is a different implementation in _pytest.pathlib.

We plan to remove this function in pytest 8.x.

Closes #11603
  • Loading branch information
hroncok committed Nov 13, 2023
1 parent 442b09e commit c3a4e83
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
7 changes: 7 additions & 0 deletions changelog/11603.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Marked the ``TestLocalPath.test_make_numbered_dir_multiprocess_safe`` test as xfail.

There is a reproducible race condition in the test and a possible
race condition in the tested ``py.path.local.make_numbered_dir`` function itself.

The tested function is not used within pytest and it is only kept
for the time being. We assume it will be removed in pytest 8.x.
3 changes: 3 additions & 0 deletions testing/_py/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,9 @@ def test_fspath_protocol_other_class(self, fake_fspath_obj):
py_path.strpath, str_path
)

@pytest.mark.xfail(
reason="#11603", strict=False, raises=(error.EEXIST, error.ENOENT)
)
def test_make_numbered_dir_multiprocess_safe(self, tmpdir):
# https://github.com/pytest-dev/py/issues/30
with multiprocessing.Pool() as pool:
Expand Down

0 comments on commit c3a4e83

Please sign in to comment.