From 361061d9005a124caadabd098909c34af4d893ed Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Thu, 18 Jan 2024 21:30:18 -0800 Subject: [PATCH 1/6] Use atexit for all multiprocessing start methods to cleanup --- Lib/multiprocessing/forkserver.py | 4 +++ Lib/multiprocessing/popen_fork.py | 4 +++ Lib/multiprocessing/process.py | 7 ++---- Lib/test/test_atexit.py | 41 ++++++++++++++++++++++++++++++- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/Lib/multiprocessing/forkserver.py b/Lib/multiprocessing/forkserver.py index 4642707dae2f4e..53b8c492675878 100644 --- a/Lib/multiprocessing/forkserver.py +++ b/Lib/multiprocessing/forkserver.py @@ -1,3 +1,4 @@ +import atexit import errno import os import selectors @@ -271,6 +272,8 @@ def sigchld_handler(*_unused): selector.close() unused_fds = [alive_r, child_w, sig_r, sig_w] unused_fds.extend(pid_to_fd.values()) + atexit._clear() + atexit.register(util._exit_function) code = _serve_one(child_r, fds, unused_fds, old_handlers) @@ -278,6 +281,7 @@ def sigchld_handler(*_unused): sys.excepthook(*sys.exc_info()) sys.stderr.flush() finally: + atexit._run_exitfuncs() os._exit(code) else: # Send pid to client process diff --git a/Lib/multiprocessing/popen_fork.py b/Lib/multiprocessing/popen_fork.py index 625981cf47627c..a57ef6bdad5ccc 100644 --- a/Lib/multiprocessing/popen_fork.py +++ b/Lib/multiprocessing/popen_fork.py @@ -1,3 +1,4 @@ +import atexit import os import signal @@ -66,10 +67,13 @@ def _launch(self, process_obj): self.pid = os.fork() if self.pid == 0: try: + atexit._clear() + atexit.register(util._exit_function) os.close(parent_r) os.close(parent_w) code = process_obj._bootstrap(parent_sentinel=child_r) finally: + atexit._run_exitfuncs() os._exit(code) else: os.close(child_w) diff --git a/Lib/multiprocessing/process.py b/Lib/multiprocessing/process.py index 271ba3fd325138..b45f7df476f7d8 100644 --- a/Lib/multiprocessing/process.py +++ b/Lib/multiprocessing/process.py @@ -310,11 +310,8 @@ def _bootstrap(self, parent_sentinel=None): # _run_after_forkers() is executed del old_process util.info('child process calling self.run()') - try: - self.run() - exitcode = 0 - finally: - util._exit_function() + self.run() + exitcode = 0 except SystemExit as e: if e.code is None: exitcode = 0 diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 913b7556be83af..9fc3911dc4da32 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -1,9 +1,11 @@ import atexit import os +import sys +import tempfile import textwrap import unittest from test import support -from test.support import script_helper +from test.support import script_helper, os_helper class GeneralTest(unittest.TestCase): @@ -46,6 +48,43 @@ def test_atexit_instances(self): self.assertEqual(res.out.decode().splitlines(), ["atexit2", "atexit1"]) self.assertFalse(res.err) + def test_multiprocessing(self): + if sys.platform == "win32": + contexts = ["spawn"] + elif sys.platform == "darwin": + contexts = ["forkserver", "spawn"] + else: + contexts = ["fork", "forkserver", "spawn"] + + with os_helper.temp_dir() as temp_dir: + output_path = os.path.join(temp_dir, 'output.txt') + code = textwrap.dedent(f""" + import multiprocessing + import os + import tempfile + + def test(arg): + import atexit + def exit_handler(): + with open('{output_path}', 'a') as f: + f.write(f'|{{arg}}|\\n') + atexit.register(exit_handler) + + if __name__ == '__main__': + for context in {contexts}: + p = multiprocessing.get_context(context).Process(target=test, args=(context,)) + p.start() + p.join() + """) + file_name = script_helper.make_script(temp_dir, 'foo', code) + script_helper.run_test_script(file_name) + + with open(output_path, "r") as f: + out = f.read() + + for context in contexts: + self.assertIn(f"|{context}|", out) + @support.cpython_only class SubinterpreterTest(unittest.TestCase): From 1a403ead72c0f846bda5c0f2de35aeac230945e0 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 19 Jan 2024 05:40:48 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-01-19-05-40-46.gh-issue-83856.jN5M80.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-01-19-05-40-46.gh-issue-83856.jN5M80.rst diff --git a/Misc/NEWS.d/next/Library/2024-01-19-05-40-46.gh-issue-83856.jN5M80.rst b/Misc/NEWS.d/next/Library/2024-01-19-05-40-46.gh-issue-83856.jN5M80.rst new file mode 100644 index 00000000000000..b2889f216a0beb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-19-05-40-46.gh-issue-83856.jN5M80.rst @@ -0,0 +1 @@ +Honor :mod:`atexit` for all :mod:`multiprocessing` start methods From 401c6919eeba08a7b1c1ab5f5b0ba943db7f9610 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Thu, 18 Jan 2024 22:27:06 -0800 Subject: [PATCH 3/6] Use raw string for the code --- Lib/test/test_atexit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 9fc3911dc4da32..3006d7f85f0ad7 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -58,7 +58,7 @@ def test_multiprocessing(self): with os_helper.temp_dir() as temp_dir: output_path = os.path.join(temp_dir, 'output.txt') - code = textwrap.dedent(f""" + code = textwrap.dedent(rf""" import multiprocessing import os import tempfile @@ -67,7 +67,7 @@ def test(arg): import atexit def exit_handler(): with open('{output_path}', 'a') as f: - f.write(f'|{{arg}}|\\n') + f.write(f'|{{arg}}|\n') atexit.register(exit_handler) if __name__ == '__main__': From 7456ad1ad1b9519cfcb4711976a08712f592e57f Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Thu, 18 Jan 2024 22:49:55 -0800 Subject: [PATCH 4/6] Use repr for path string --- Lib/test/test_atexit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 3006d7f85f0ad7..c5c28e0da5cc72 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -66,7 +66,7 @@ def test_multiprocessing(self): def test(arg): import atexit def exit_handler(): - with open('{output_path}', 'a') as f: + with open({repr(output_path)}, 'a') as f: f.write(f'|{{arg}}|\n') atexit.register(exit_handler) From 48b2da8d6045be517a503517b1d32f2e3de4d028 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Mon, 15 Apr 2024 20:19:07 -0700 Subject: [PATCH 5/6] Move tests from test_atexit to _test_multiprocessing --- Lib/test/_test_multiprocessing.py | 23 +++++++++++++++++++ Lib/test/test_atexit.py | 37 ------------------------------- 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 8e4e0765d46809..c8ef103f49bdda 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6083,6 +6083,29 @@ def submain(): pass self.assertFalse(err, msg=err.decode('utf-8')) +class _TestAtExit(BaseTestCase): + + ALLOWED_TYPES = ('processes',) + + @classmethod + def _write_file_at_exit(self, output_path): + import atexit + def exit_handler(): + with open(output_path, 'w') as f: + f.write("deadbeef") + atexit.register(exit_handler) + + def test_atexit(self): + # gh-83856 + with os_helper.temp_dir() as temp_dir: + output_path = os.path.join(temp_dir, 'output.txt') + p = self.Process(target=self._write_file_at_exit, args=(output_path,)) + p.start() + p.join() + with open(output_path) as f: + self.assertEqual(f.read(), 'deadbeef') + + class MiscTestCase(unittest.TestCase): def test__all__(self): # Just make sure names in not_exported are excluded diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index c5c28e0da5cc72..7b63754cf5840d 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -48,43 +48,6 @@ def test_atexit_instances(self): self.assertEqual(res.out.decode().splitlines(), ["atexit2", "atexit1"]) self.assertFalse(res.err) - def test_multiprocessing(self): - if sys.platform == "win32": - contexts = ["spawn"] - elif sys.platform == "darwin": - contexts = ["forkserver", "spawn"] - else: - contexts = ["fork", "forkserver", "spawn"] - - with os_helper.temp_dir() as temp_dir: - output_path = os.path.join(temp_dir, 'output.txt') - code = textwrap.dedent(rf""" - import multiprocessing - import os - import tempfile - - def test(arg): - import atexit - def exit_handler(): - with open({repr(output_path)}, 'a') as f: - f.write(f'|{{arg}}|\n') - atexit.register(exit_handler) - - if __name__ == '__main__': - for context in {contexts}: - p = multiprocessing.get_context(context).Process(target=test, args=(context,)) - p.start() - p.join() - """) - file_name = script_helper.make_script(temp_dir, 'foo', code) - script_helper.run_test_script(file_name) - - with open(output_path, "r") as f: - out = f.read() - - for context in contexts: - self.assertIn(f"|{context}|", out) - @support.cpython_only class SubinterpreterTest(unittest.TestCase): From a24b9f138403d29480421792eab079fb59873b77 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Mon, 15 Apr 2024 20:19:45 -0700 Subject: [PATCH 6/6] Remove unused import --- Lib/test/test_atexit.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 7b63754cf5840d..913b7556be83af 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -1,11 +1,9 @@ import atexit import os -import sys -import tempfile import textwrap import unittest from test import support -from test.support import script_helper, os_helper +from test.support import script_helper class GeneralTest(unittest.TestCase):