]> granicus.if.org Git - python/commitdiff
[3.6] bpo-30028: make test.support.temp_cwd() fork-safe (GH-1066) (GH-5826)
authorAnselm Kruis <a.kruis@science-computing.de>
Fri, 23 Feb 2018 16:27:54 +0000 (17:27 +0100)
committerGregory P. Smith <greg@krypto.org>
Fri, 23 Feb 2018 16:27:54 +0000 (08:27 -0800)
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 33dddac00ba8d9b72cf21b8698504077eb3c23ad)

Co-authored-by: Anselm Kruis <a.kruis@science-computing.de>
Lib/test/support/__init__.py
Lib/test/test_support.py
Misc/ACKS

index 2930ab24e4d047862896ab028be7a15acd71fdb8..e4840e6a9618d7db6d34f27d8967a9ca34f61e74 100644 (file)
@@ -952,10 +952,14 @@ def temp_dir(path=None, quiet=False):
                 raise
             warnings.warn('tests may fail, unable to create temp dir: ' + path,
                           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
index c070809cbe9bf684c48cdf5c078b1d2304bf3867..fff0fc742f3655a49319fbebfb5c6e733e1d1597 100644 (file)
@@ -6,8 +6,10 @@ import os
 import unittest
 import socket
 import tempfile
+import textwrap
 import errno
 from test import support
+from test.support import script_helper
 
 TESTFN = support.TESTFN
 
@@ -158,6 +160,33 @@ class TestSupport(unittest.TestCase):
         expected = ['tests may fail, unable to create temp dir: ' + path]
         self.assertEqual(warnings, expected)
 
+    @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):
index 2a7b1b8140a4f36eda6c42e43c481f177c30149c..75c9b3f0563de113174cc9c7ec9d8b2bf3324c22 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -841,6 +841,7 @@ Pedro Kroger
 Hannu Krosing
 Andrej Krpic
 Ivan Krstić
+Anselm Kruis
 Steven Kryskalla
 Andrew Kuchling
 Dave Kuhlman