Skip to content

Commit 61bd4d2

Browse files
Anselm Kruisgpshead
authored andcommitted
[2.7] bpo-30028: make test.support.temp_cwd() fork-safe (GH-1066) (GH-5825)
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 b852d8c commit 61bd4d2

File tree

3 files changed

+36
-1
lines changed

3 files changed

+36
-1
lines changed

Lib/test/support/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,10 +753,14 @@ def temp_dir(path=None, quiet=False):
753753
raise
754754
warnings.warn('tests may fail, unable to create temp dir: ' + path,
755755
RuntimeWarning, stacklevel=3)
756+
if dir_created:
757+
pid = os.getpid()
756758
try:
757759
yield path
758760
finally:
759-
if dir_created:
761+
# In case the process forks, let only the parent remove the
762+
# directory. The child has a diffent process id. (bpo-30028)
763+
if dir_created and pid == os.getpid():
760764
rmtree(path)
761765

762766
@contextlib.contextmanager

Lib/test/test_test_support.py

Lines changed: 30 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

@@ -176,6 +178,34 @@ def test_temp_dir__existing_dir__quiet_true(self):
176178
expected = ['tests may fail, unable to create temp dir: ' + path]
177179
self.assertEqual(warnings, expected)
178180

181+
@unittest.skipUnless(hasattr(os, "fork"), "test requires os.fork")
182+
def test_temp_dir__forked_child(self):
183+
"""Test that a forked child process does not remove the directory."""
184+
# See bpo-30028 for details.
185+
# Run the test as an external script, because it uses fork.
186+
script_helper.assert_python_ok("-c", textwrap.dedent("""
187+
import os
188+
from test import support
189+
with support.temp_cwd() as temp_path:
190+
pid = os.fork()
191+
if pid != 0:
192+
# parent process (child has pid == 0)
193+
194+
# wait for the child to terminate
195+
(pid, status) = os.waitpid(pid, 0)
196+
if status != 0:
197+
raise AssertionError("Child process failed with exit "
198+
"status indication "
199+
"0x{:x}.".format(status))
200+
201+
# Make sure that temp_path is still present. When the child
202+
# process leaves the 'temp_cwd'-context, the __exit__()-
203+
# method of the context must not remove the temporary
204+
# directory.
205+
if not os.path.isdir(temp_path):
206+
raise AssertionError("Child removed temp_path.")
207+
"""))
208+
179209
# Tests for change_cwd()
180210

181211
def test_change_cwd(self):

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,7 @@ Pedro Kroger
774774
Hannu Krosing
775775
Andrej Krpic
776776
Ivan Krstić
777+
Anselm Kruis
777778
Steven Kryskalla
778779
Andrew Kuchling
779780
Dave Kuhlman

0 commit comments

Comments
 (0)