[Python-checkins] cpython (3.5): Issue #25654:

[victor.stinner]

https://hg.python.org/cpython/rev/357d1ed928fe
changeset:   100739:357d1ed928fe
branch:      3.5
user:        Victor Stinner <victor.stinner at gmail.com>
date:        Fri Mar 25 09:29:50 2016 +0100
summary:
  Issue #25654:

* multiprocessing: open file with closefd=False to avoid ResourceWarning
* _test_multiprocessing: open file with O_EXCL to detect bugs in tests (if a
  previous test forgot to remove TESTFN)
* test_sys_exit(): remove TESTFN after each loop iteration

Initial patch written by Serhiy Storchaka.

files:
  Doc/library/multiprocessing.rst   |   2 +-
  Lib/multiprocessing/forkserver.py |   8 +-----
  Lib/multiprocessing/process.py    |   7 +----
  Lib/multiprocessing/util.py       |  23 +++++++++++++++++++
  Lib/test/_test_multiprocessing.py |  18 ++++++++++----
  5 files changed, 39 insertions(+), 19 deletions(-)


diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst
--- a/Doc/library/multiprocessing.rst
+++ b/Doc/library/multiprocessing.rst
@@ -2689,7 +2689,7 @@
     in issues with processes-in-processes. This has been changed to::
 
         sys.stdin.close()
-        sys.stdin = open(os.devnull)
+        sys.stdin = open(os.open(os.devnull, os.O_RDONLY), closefd=False)
 
     Which solves the fundamental issue of processes colliding with each other
     resulting in a bad file descriptor error, but introduces a potential danger
diff --git a/Lib/multiprocessing/forkserver.py b/Lib/multiprocessing/forkserver.py
--- a/Lib/multiprocessing/forkserver.py
+++ b/Lib/multiprocessing/forkserver.py
@@ -147,13 +147,7 @@
             except ImportError:
                 pass
 
-    # close sys.stdin
-    if sys.stdin is not None:
-        try:
-            sys.stdin.close()
-            sys.stdin = open(os.devnull)
-        except (OSError, ValueError):
-            pass
+    util._close_stdin()
 
     # ignoring SIGCHLD means no need to reap zombie processes
     handler = signal.signal(signal.SIGCHLD, signal.SIG_IGN)
diff --git a/Lib/multiprocessing/process.py b/Lib/multiprocessing/process.py
--- a/Lib/multiprocessing/process.py
+++ b/Lib/multiprocessing/process.py
@@ -234,12 +234,7 @@
                 context._force_start_method(self._start_method)
             _process_counter = itertools.count(1)
             _children = set()
-            if sys.stdin is not None:
-                try:
-                    sys.stdin.close()
-                    sys.stdin = open(os.devnull)
-                except (OSError, ValueError):
-                    pass
+            util._close_stdin()
             old_process = _current_process
             _current_process = self
             try:
diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py
--- a/Lib/multiprocessing/util.py
+++ b/Lib/multiprocessing/util.py
@@ -9,6 +9,7 @@
 
 import os
 import itertools
+import sys
 import weakref
 import atexit
 import threading        # we want threading to install it's
@@ -356,6 +357,28 @@
     assert fds[-1] == MAXFD, 'fd too large'
     for i in range(len(fds) - 1):
         os.closerange(fds[i]+1, fds[i+1])
+#
+# Close sys.stdin and replace stdin with os.devnull
+#
+
+def _close_stdin():
+    if sys.stdin is None:
+        return
+
+    try:
+        sys.stdin.close()
+    except (OSError, ValueError):
+        pass
+
+    try:
+        fd = os.open(os.devnull, os.O_RDONLY)
+        try:
+            sys.stdin = open(fd, closefd=False)
+        except:
+            os.close(fd)
+            raise
+    except (OSError, ValueError):
+        pass
 
 #
 # Start a program with only specified fds kept open
diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py
--- a/Lib/test/_test_multiprocessing.py
+++ b/Lib/test/_test_multiprocessing.py
@@ -455,13 +455,15 @@
 
     @classmethod
     def _test_stderr_flush(cls, testfn):
-        sys.stderr = open(testfn, 'w')
+        fd = os.open(testfn, os.O_WRONLY | os.O_CREAT | os.O_EXCL)
+        sys.stderr = open(fd, 'w', closefd=False)
         1/0 # MARKER
 
 
     @classmethod
     def _test_sys_exit(cls, reason, testfn):
-        sys.stderr = open(testfn, 'w')
+        fd = os.open(testfn, os.O_WRONLY | os.O_CREAT | os.O_EXCL)
+        sys.stderr = open(fd, 'w', closefd=False)
         sys.exit(reason)
 
     def test_sys_exit(self):
@@ -472,15 +474,21 @@
         testfn = test.support.TESTFN
         self.addCleanup(test.support.unlink, testfn)
 
-        for reason, code in (([1, 2, 3], 1), ('ignore this', 1)):
+        for reason in (
+            [1, 2, 3],
+            'ignore this',
+        ):
             p = self.Process(target=self._test_sys_exit, args=(reason, testfn))
             p.daemon = True
             p.start()
             p.join(5)
-            self.assertEqual(p.exitcode, code)
+            self.assertEqual(p.exitcode, 1)
 
             with open(testfn, 'r') as f:
-                self.assertEqual(f.read().rstrip(), str(reason))
+                content = f.read()
+            self.assertEqual(content.rstrip(), str(reason))
+
+            os.unlink(testfn)
 
         for reason in (True, False, 8):
             p = self.Process(target=sys.exit, args=(reason,))

-- 
Repository URL: https://hg.python.org/cpython