[BUG] Since setuptools 75.0.0, packages fail to build with race condition: "No such file or directory"

[eli-schwartz]

setuptools version

setuptools==75.0.0

Python version

Python 3.12

OS

Gentoo Linux

Additional environment information

No response

Description

https://bugs.gentoo.org/939798
https://bugs.gentoo.org/939875

Packages fail to build with "no such file or directory" claiming that compiler output files cannot be created.

Expected behavior

Setuptools should create directories needed by an output file before running the compilation.

How to Reproduce

Build cython with job parallelism and cross your fingers.

Output

2024-09-19 21:23:05,173 root INFO running build_ext
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Tempita/_tempita.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Utils.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/FusedNode.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/Code.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/Visitor.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/FlowControl.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex/Scanners.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/Scanning.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Compiler/Parsing.py
[1/1] Cythonizing /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex/Machines.py
2024-09-19 21:23:13,316 root INFO building 'Cython.Plex.Scanners' extension
2024-09-19 21:23:13,372 root INFO creating /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11-python3_12/build/temp.linux-x86_64-cpython-312/var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex
2024-09-19 21:23:13,868 root INFO building 'Cython.Plex.Machines' extension
2024-09-19 21:23:13,869 root INFO x86_64-pc-linux-gnu-gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -O2 -pipe -ggdb -DNDEBUG -fPIC -I/usr/include/python3.12 -c /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex/Machines.c -o /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11-python3_12/build/temp.linux-x86_64-cpython-312/var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex/Machines.o
Assembler messages:                                                                                                                                                                                                 
Fatal error: can't create /var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11-python3_12/build/temp.linux-x86_64-cpython-312/var/tmp/portage/dev-python/cython-3.0.11/work/cython-3.0.11/Cython/Plex/Machines.o: No such file or directory

[eli-schwartz]

f2157f5

This commit in distutils looks incredibly suspicious. I cannot see how it would be functional. It replaces a global cache variable paired with a function that:

• checks if a directory has previously been created, or return early
• runs Path.mkdir()
• upon success, updates the global cache

with new logic that does this:

• check if a directory has previously been created, or return early
• update the global cache
• returns a function to call Path.mkdir()

This is not going to be parallel-safe.

[eli-schwartz]

Ping. It is currently impossible to upgrade setuptools due to this bug, and it's not clear what we should do about this.

[mgorny]

@abravalheri, any chance you could address it? Reverting the distutils commit should be a good enough solution for the time being.

[abravalheri]

If this comes from a distutils commit, we probably have to address it there first (we don't change the _distutils directory directly in this repo, we only merge from pypa/distutils).

@jaraco should we move this issue to pypa/distutils?

[jaraco]

Thanks for the report and for the analysis in #304 (comment).

f2157f5

This commit in distutils looks incredibly suspicious. I cannot see how it would be functional. It replaces a global cache variable paired with a function that:

• checks if a directory has previously been created, or return early
• runs Path.mkdir()
• upon success, updates the global cache

with new logic that does this:

• check if a directory has previously been created, or return early
• update the global cache
• returns a function to call Path.mkdir()

This is not going to be parallel-safe.

I don't think the new logic characterization is quite fair. The new logic doesn't return a function to call Path.mkdir(), but like the previous logic, runs Path.mkdir().

So the only difference is the order of operations (whether the cache is updated before or after mkdir runs), and whether the cache is updated on a failure. Perhaps all that's needed to correct the behavior is to address that concern.

[jaraco]

Before I cut a release with Setuptools, would either of you be willing to confirm that by applying 5904204 to an affected environment that it addresses the issue?

[eli-schwartz]

I just got back online after the holiday and haven't had a chance to test it (yet).

But I will say that based on the commit message alone, the regression test is wrong (and the fix might be wrong, but I'm not sure). After mkpath is run, the attempt was a success and the path is guaranteed to exist -- the problem isn't that failures may be cached, the problem is that the cache happens first, and then the success happens, so in a multi threaded application such as distutils (or setuptools),

• mkpath will cache a success
• another parallel job retrieves the cache and doesn't do anything, then errors out
• the first job succeeds at creating the path and also succeeds at the overall threaded job

A test case testing for OSError doesn't simulate the thread conditions that failed in the bug report. I agree it's an additional failure mode.

I'm also inclined to agree without a chance to test it, that it's the same failure mode and a fix for one will fix both.

So I think the answer is that the setup.py -j$(nproc) case is still being untested and may continue to regress in different ways even though this regression has been solved.

[jaraco]

Yes, I agree the proposed fix only addresses the "order of operations" failure mode and doesn't address issues of thread safety or other parallelism. I chose that approach because I believe that after correcting the "order of operations" concern, the behavior should be functionally the same as it was before the regression. That is, I've been unable to identify any way in which the code prior to f2157f5 would not have had the same parallelism risks, as both use a single global cache within the dir_util module.

So I think the answer is that the setup.py -j$(nproc) case is still being untested and may continue to regress in different ways even though this regression has been solved.

Yes, I agree, but I'm thinking that creating a proper regression test for this case would be difficult and brittle, especially since it's a race condition, so subject to machine performance and other measures. Without a minimal reproducer, it's difficult to even conceive of a regression test. That's why I'm taking a pragmatic approach to restore the prior behavior in the hopes that will be sufficient to unblock the issue.

I'd love to see a more robust test that captures failures in parallelism, perhaps more broadly across setuptools, but that's a much more ambitious ask and probably lower priority than some other open concerns. Still, if you feel strongly about it or especially if you'd like to contribute such a fix, I'd welcome you to file an issue with setuptools (or possibly here, but probably setuptools) to track the broader issue.