From 8a208510104a0ac5a34240ae4329b881ffcee998 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 25 Mar 2016 09:29:50 +0100 Subject: 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. --- 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 index 8209ae9..684a59f 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -2689,7 +2689,7 @@ Beware of replacing :data:`sys.stdin` with a "file like object" 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 index b27cba5..ad01ede 100644 --- a/Lib/multiprocessing/forkserver.py +++ b/Lib/multiprocessing/forkserver.py @@ -147,13 +147,7 @@ def main(listener_fd, alive_r, preload, main_path=None, sys_path=None): 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 index 68959bf..bca8b7a 100644 --- a/Lib/multiprocessing/process.py +++ b/Lib/multiprocessing/process.py @@ -234,12 +234,7 @@ class BaseProcess(object): 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 index ea5443d..1a2c0db 100644 --- 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 @@ def close_all_fds_except(fds): 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 index 1bbbd0b..11a6310 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -455,13 +455,15 @@ class _TestSubclassingProcess(BaseTestCase): @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 @@ class _TestSubclassingProcess(BaseTestCase): 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,)) -- cgit v0.12