From f202fff5a5a4a8ef1914748f446fc1a34515ea73 Mon Sep 17 00:00:00 2001 From: XD Trol Date: Sun, 16 Jan 2022 02:11:21 +0800 Subject: [PATCH 1/3] bpo-46391: Library multiprocess leaks named resources. --- Lib/multiprocessing/context.py | 14 ++++++++++ Lib/multiprocessing/process.py | 10 +++++-- Lib/test/_test_multiprocessing.py | 27 +++++++++++++++++++ .../2022-01-16-03-09-58.bpo-46391.LZUzDI.rst | 2 ++ 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst diff --git a/Lib/multiprocessing/context.py b/Lib/multiprocessing/context.py index 8d0525d5d62179..b1960ea296fe20 100644 --- a/Lib/multiprocessing/context.py +++ b/Lib/multiprocessing/context.py @@ -223,6 +223,10 @@ class Process(process.BaseProcess): def _Popen(process_obj): return _default_context.get_context().Process._Popen(process_obj) + @staticmethod + def _after_fork(): + return _default_context.get_context().Process._after_fork() + class DefaultContext(BaseContext): Process = Process @@ -283,6 +287,11 @@ def _Popen(process_obj): from .popen_spawn_posix import Popen return Popen(process_obj) + @staticmethod + def _after_fork(): + # process is spawned, nothing to do + pass + class ForkServerProcess(process.BaseProcess): _start_method = 'forkserver' @staticmethod @@ -326,6 +335,11 @@ def _Popen(process_obj): from .popen_spawn_win32 import Popen return Popen(process_obj) + @staticmethod + def _after_fork(): + # process is spawned, nothing to do + pass + class SpawnContext(BaseContext): _name = 'spawn' Process = SpawnProcess diff --git a/Lib/multiprocessing/process.py b/Lib/multiprocessing/process.py index 3917d2e4fa63ec..c03c859baa795b 100644 --- a/Lib/multiprocessing/process.py +++ b/Lib/multiprocessing/process.py @@ -304,8 +304,7 @@ def _bootstrap(self, parent_sentinel=None): if threading._HAVE_THREAD_NATIVE_ID: threading.main_thread()._set_native_id() try: - util._finalizer_registry.clear() - util._run_after_forkers() + self._after_fork() finally: # delay finalization of the old process object until after # _run_after_forkers() is executed @@ -336,6 +335,13 @@ def _bootstrap(self, parent_sentinel=None): return exitcode + @staticmethod + def _after_fork(): + from . import util + util._finalizer_registry.clear() + util._run_after_forkers() + + # # We subclass bytes to avoid accidental transmission of auth keys over network # diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 67bb17c0ede364..fed7f820e74354 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5760,6 +5760,33 @@ def test_namespace(self): self.run_worker(self._test_namespace, o) +class TestNamedResource(unittest.TestCase): + def test_global_named_resource_spawn(self): + # + # Check that global named resources in main module + # will not leak by a subprocess, in spawn context. + # + with os_helper.temp_dir() as tmp_dir: + source = os.path.join(tmp_dir, 'source.py') + with open(source, 'w') as f: + f.write('''if 1: + import multiprocessing as mp + + ctx = mp.get_context('spawn') + + global_resource = ctx.Semaphore() + + def submain(): pass + + if __name__ == '__main__': + p = ctx.Process(target=submain) + p.start() + p.join() + ''') + rc, out, err = test.support.script_helper.assert_python_ok(source) + self.assertNotRegex(err, b'resource_tracker: There appear to be .* leaked') + + class MiscTestCase(unittest.TestCase): def test__all__(self): # Just make sure names in not_exported are excluded diff --git a/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst b/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst new file mode 100644 index 00000000000000..edc67a97232e40 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst @@ -0,0 +1,2 @@ +Fix that Library multiprocess leaks named resources when global named +resources are used in the main module in spawn context. From f3a947d80677929ee73b3e17e4e205779d1d97de Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 6 Jun 2022 12:34:39 +0200 Subject: [PATCH 2/3] Improve NEWS wording --- .../next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst b/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst index edc67a97232e40..6ebdc394900e63 100644 --- a/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst +++ b/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst @@ -1,2 +1,2 @@ -Fix that Library multiprocess leaks named resources when global named -resources are used in the main module in spawn context. +Fix a multiprocessing bug where a global named resource (such as a semaphore) +could leak when a child process is spawned (as opposed to forked). From 7cc22aef34aa303f148b630eca16937d161361d6 Mon Sep 17 00:00:00 2001 From: XD Trol Date: Tue, 7 Jun 2022 16:08:45 +0800 Subject: [PATCH 3/3] update test code and news --- Lib/test/_test_multiprocessing.py | 35 ++++++++++--------- ...-06-07-14-53-46.gh-issue-90549.T4FMKY.rst} | 0 2 files changed, 19 insertions(+), 16 deletions(-) rename Misc/NEWS.d/next/Library/{2022-01-16-03-09-58.bpo-46391.LZUzDI.rst => 2022-06-07-14-53-46.gh-issue-90549.T4FMKY.rst} (100%) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index fed7f820e74354..4a588d96deb943 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5,6 +5,7 @@ import unittest import unittest.mock import queue as pyqueue +import textwrap import time import io import itertools @@ -5763,28 +5764,30 @@ def test_namespace(self): class TestNamedResource(unittest.TestCase): def test_global_named_resource_spawn(self): # - # Check that global named resources in main module + # gh-90549: Check that global named resources in main module # will not leak by a subprocess, in spawn context. # - with os_helper.temp_dir() as tmp_dir: - source = os.path.join(tmp_dir, 'source.py') - with open(source, 'w') as f: - f.write('''if 1: - import multiprocessing as mp + testfn = os_helper.TESTFN + self.addCleanup(os_helper.unlink, testfn) + with open(testfn, 'w', encoding='utf-8') as f: + f.write(textwrap.dedent('''\ + import multiprocessing as mp - ctx = mp.get_context('spawn') + ctx = mp.get_context('spawn') - global_resource = ctx.Semaphore() + global_resource = ctx.Semaphore() - def submain(): pass + def submain(): pass - if __name__ == '__main__': - p = ctx.Process(target=submain) - p.start() - p.join() - ''') - rc, out, err = test.support.script_helper.assert_python_ok(source) - self.assertNotRegex(err, b'resource_tracker: There appear to be .* leaked') + if __name__ == '__main__': + p = ctx.Process(target=submain) + p.start() + p.join() + ''')) + rc, out, err = test.support.script_helper.assert_python_ok(testfn) + # on error, err = 'UserWarning: resource_tracker: There appear to + # be 1 leaked semaphore objects to clean up at shutdown' + self.assertEqual(err, b'') class MiscTestCase(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst b/Misc/NEWS.d/next/Library/2022-06-07-14-53-46.gh-issue-90549.T4FMKY.rst similarity index 100% rename from Misc/NEWS.d/next/Library/2022-01-16-03-09-58.bpo-46391.LZUzDI.rst rename to Misc/NEWS.d/next/Library/2022-06-07-14-53-46.gh-issue-90549.T4FMKY.rst