Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XFAIL TestLocalPath.test_make_numbered_dir_multiprocess_safe #11611

Conversation

hroncok
Copy link
Member

@hroncok hroncok commented Nov 13, 2023

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 function 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

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hroncok!

@hroncok hroncok force-pushed the xfail_test_make_numbered_dir_multiprocess_safe branch from 8f1f675 to c067bb3 Compare November 14, 2023 10:15
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 pytest-dev#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 pytest-dev#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 module eventually anyway.

Closes pytest-dev#11603
@hroncok hroncok force-pushed the xfail_test_make_numbered_dir_multiprocess_safe branch from c067bb3 to 052d145 Compare November 14, 2023 10:18
@hroncok
Copy link
Member Author

hroncok commented Nov 14, 2023

I also amended the commit message not to say we will remove this function from 8.x.

@nicoddemus nicoddemus merged commit 223e030 into pytest-dev:main Nov 14, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestLocalPath.test_make_numbered_dir_multiprocess_safe sometimes fails with py.error.EEXIST: [File exists]
2 participants