diff options
author | Anselm Kruis <a.kruis@science-computing.de> | 2018-02-23 01:37:38 (GMT) |
---|---|---|
committer | Gregory P. Smith <greg@krypto.org> | 2018-02-23 01:37:38 (GMT) |
commit | 33dddac00ba8d9b72cf21b8698504077eb3c23ad (patch) | |
tree | 40893955f445131d96c340493afd027cfcaa66f5 | |
parent | 520b7ae27e39d1c77ea74ccd1b184d7cb43f9dcb (diff) | |
download | cpython-33dddac00ba8d9b72cf21b8698504077eb3c23ad.zip cpython-33dddac00ba8d9b72cf21b8698504077eb3c23ad.tar.gz cpython-33dddac00ba8d9b72cf21b8698504077eb3c23ad.tar.bz2 |
bpo-30028: make test.support.temp_cwd() fork-safe (GH-1066)
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.
-rw-r--r-- | Lib/test/support/__init__.py | 6 | ||||
-rw-r--r-- | Lib/test/test_support.py | 29 | ||||
-rw-r--r-- | Misc/ACKS | 1 |
3 files changed, 35 insertions, 1 deletions
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 6c9e31a..b4269f4 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -948,10 +948,14 @@ def temp_dir(path=None, quiet=False): warnings.warn(f'tests may fail, unable to create ' f'temporary directory {path!r}: {exc}', RuntimeWarning, stacklevel=3) + if dir_created: + pid = os.getpid() try: yield path finally: - if dir_created: + # In case the process forks, let only the parent remove the + # directory. The child has a diffent process id. (bpo-30028) + if dir_created and pid == os.getpid(): rmtree(path) @contextlib.contextmanager diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index e06f7b8..36d5f84 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -9,9 +9,11 @@ import stat import subprocess import sys import tempfile +import textwrap import time import unittest from test import support +from test.support import script_helper TESTFN = support.TESTFN @@ -165,6 +167,33 @@ class TestSupport(unittest.TestCase): f'temporary directory {path!r}: '), warn) + @unittest.skipUnless(hasattr(os, "fork"), "test requires os.fork") + def test_temp_dir__forked_child(self): + """Test that a forked child process does not remove the directory.""" + # See bpo-30028 for details. + # Run the test as an external script, because it uses fork. + script_helper.assert_python_ok("-c", textwrap.dedent(""" + import os + from test import support + with support.temp_cwd() as temp_path: + pid = os.fork() + if pid != 0: + # parent process (child has pid == 0) + + # wait for the child to terminate + (pid, status) = os.waitpid(pid, 0) + if status != 0: + raise AssertionError(f"Child process failed with exit " + f"status indication 0x{status:x}.") + + # Make sure that temp_path is still present. When the child + # process leaves the 'temp_cwd'-context, the __exit__()- + # method of the context must not remove the temporary + # directory. + if not os.path.isdir(temp_path): + raise AssertionError("Child removed temp_path.") + """)) + # Tests for change_cwd() def test_change_cwd(self): @@ -857,6 +857,7 @@ Pedro Kroger Hannu Krosing Andrej Krpic Ivan Krstić +Anselm Kruis Steven Kryskalla Andrew Kuchling Dave Kuhlman |