gh-157579: Fix race condition in the cleanup of tempfile.TemporaryDirectory

[StanFromIreland]

• Issue: Race condition in the cleanup of tempfile.TemporaryDirectory #157579

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #34695934 | 📁 Comparing 7568089 against main (82952e3)

  🔍 Preview build  

18 files changed · ± 18 modified

± Modified

• license.html
• c-api/bytes.html
• c-api/unicode.html
• library/asyncio-task.html
• library/ctypes.html
• library/importlib.html
• library/ipaddress.html
• library/json.html
• library/pathlib.html
• library/statistics.html
• and 8 more...

[StanFromIreland]

I'm not sure why Emscripten failed in this run on test_cleanup_with_symlink_race:

 ======================================================================
FAIL: test_cleanup_with_symlink_race (test.test_tempfile.TestTemporaryDirectory.test_cleanup_with_symlink_race)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/test/test_tempfile.py", line 1907, in test_cleanup_with_symlink_race
    self.assertTrue(os.path.exists(target_file))
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False is not true

The test swaps a directory for a symlink to the outside directory mid-recovery, however the unlink followed the symlink and deleted it. This is odd because the unlink was passed a dir_fd for the directory that got swapped out. However, it doesn’t hit our skip (so the platform meets the requirements, which means os.unlink is indeed in os.supports_dir_fd). CC @hoodmane any idea what's going on here?

[hoodmane]

Will take a look.

[encukou]

Meanwhile let's start buildbots for all the other architectures/configurations

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @encukou for commit a837624 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F157580%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[StanFromIreland]

And it seems all the buildbots older with masOS <26 (and iOS) are failing on test_cleanup_with_symlink_race with:

======================================================================
ERROR: test_cleanup_with_symlink_race (test.test_tempfile.TestTemporaryDirectory.test_cleanup_with_symlink_race)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/buildarea/pull_request.itamaro-macos-arm64-aws.macos-with-brew.nogil/build/Lib/test/test_tempfile.py", line 1904, in test_cleanup_with_symlink_race
    os.chmod(dir1, 0o700)
    ~~~~~~~~^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/test_python_lxxiashb/3w36qc28/dir1'

We don't know if the test passes or not, as we don't reach the final assertions. I'm also not quite sure why this happens, and I don't have access to a machine to debug. It's possible that the hook's rename of the 0o500 directory fails? Any ideas Petr?

[encukou]

Funny story

I can reproduce! .... on an ancient Intel MacBook that bought it in this state to take macOS screenshots for tutorials. It lasts ~10 min on battery and has a broken screen. It also has only has 1 USB port, so I can charge it, or connect a monitor/LAN, but not both at once. (This was never a problem for screenshots...)

So, work is slow. But, in the end, after several tries, I did manage to find a compatible dongle.

Turns out that on old macOS, you can't rename a directory if you don't have write permissions on that directory. (On Linux and -- apparently -- new macs, you need write on the containing directory; that has the inode you're manipulating.)

---

The rmtree will:

• open d1 & start scandir
• remove test0.txt from d1 via the monkeypatched hook; this succeeds
• open dir1 & start scandir
• remove file1 from dir1 via the monkeypatched hook
  • remove file1 from dir1; this fails with PermissionError
  • rename dir1 to dir1_moved; this also fails due to PermissionError (!)
• call onexc with hook as func, file1 as path, dir1's fd as dir_fd
  • call chflags on dir1
  • call chmod on dir1's fd
  • call chflags on file1
  • call chmod on file1 via dir1's fd
  • remove file1 (via the hook; this succeeds)
• remove dir1 via d1's fd
• rmdir d1 via path

Back in the test, in the finally:

• islink is False as dir1 doesn't exist
• chmod fails as dir1 doesn't exist

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

[miss-islington-app]

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @StanFromIreland and @encukou, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 5c20517a4fc56683efe63a7751020db9573f538d 3.12

[miss-islington-app]

Sorry, @StanFromIreland and @encukou, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 5c20517a4fc56683efe63a7751020db9573f538d 3.11

[miss-islington-app]

Sorry, @StanFromIreland and @encukou, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 5c20517a4fc56683efe63a7751020db9573f538d 3.10

[bedevere-app]

GH-158429 is a backport of this pull request to the 3.13 branch.

[bedevere-app]

GH-158430 is a backport of this pull request to the 3.14 branch.

[bedevere-app]

GH-158431 is a backport of this pull request to the 3.15 branch.

[bedevere-app]

GH-158489 is a backport of this pull request to the 3.12 branch.