Skip to content

Commit 9c819a6

Browse files
Anselm Kruisgpshead
authored andcommitted
[3.6] bpo-30028: make test.support.temp_cwd() fork-safe (GH-1066) (GH-5826)
Make test.support.temp_cwd() fork-safe. The context manager test.support.temp_cwd() no longer removes the temporary directory when executing in a process other than the parent it entered from. If a forked child exits the context manager it won't do the cleanup.. (cherry picked from commit 33dddac) Co-authored-by: Anselm Kruis <[email protected]>
1 parent dd52d5c commit 9c819a6

File tree

3 files changed

+35
-1
lines changed

3 files changed

+35
-1
lines changed

Lib/test/support/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -952,10 +952,14 @@ def temp_dir(path=None, quiet=False):
952952
raise
953953
warnings.warn('tests may fail, unable to create temp dir: ' + path,
954954
RuntimeWarning, stacklevel=3)
955+
if dir_created:
956+
pid = os.getpid()
955957
try:
956958
yield path
957959
finally:
958-
if dir_created:
960+
# In case the process forks, let only the parent remove the
961+
# directory. The child has a diffent process id. (bpo-30028)
962+
if dir_created and pid == os.getpid():
959963
rmtree(path)
960964

961965
@contextlib.contextmanager

Lib/test/test_support.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
import unittest
77
import socket
88
import tempfile
9+
import textwrap
910
import errno
1011
from test import support
12+
from test.support import script_helper
1113

1214
TESTFN = support.TESTFN
1315

@@ -158,6 +160,33 @@ def test_temp_dir__existing_dir__quiet_true(self):
158160
expected = ['tests may fail, unable to create temp dir: ' + path]
159161
self.assertEqual(warnings, expected)
160162

163+
@unittest.skipUnless(hasattr(os, "fork"), "test requires os.fork")
164+
def test_temp_dir__forked_child(self):
165+
"""Test that a forked child process does not remove the directory."""
166+
# See bpo-30028 for details.
167+
# Run the test as an external script, because it uses fork.
168+
script_helper.assert_python_ok("-c", textwrap.dedent("""
169+
import os
170+
from test import support
171+
with support.temp_cwd() as temp_path:
172+
pid = os.fork()
173+
if pid != 0:
174+
# parent process (child has pid == 0)
175+
176+
# wait for the child to terminate
177+
(pid, status) = os.waitpid(pid, 0)
178+
if status != 0:
179+
raise AssertionError(f"Child process failed with exit "
180+
f"status indication 0x{status:x}.")
181+
182+
# Make sure that temp_path is still present. When the child
183+
# process leaves the 'temp_cwd'-context, the __exit__()-
184+
# method of the context must not remove the temporary
185+
# directory.
186+
if not os.path.isdir(temp_path):
187+
raise AssertionError("Child removed temp_path.")
188+
"""))
189+
161190
# Tests for change_cwd()
162191

163192
def test_change_cwd(self):

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,7 @@ Pedro Kroger
841841
Hannu Krosing
842842
Andrej Krpic
843843
Ivan Krstić
844+
Anselm Kruis
844845
Steven Kryskalla
845846
Andrew Kuchling
846847
Dave Kuhlman

0 commit comments

Comments
 (0)