From abb1037d0961fb4e1023926976a4f24c0a09ded5 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Wed, 5 Jul 2023 13:12:50 -0700 Subject: [PATCH 1/9] gh-90876: Prevent a multiprocessing import error. Prevent `multiprocessing.spawn` from failing to *import* in environments where `sys.executable` is `None`. This regressed in 3.11 with the addition of support for path-like objects in multiprocessing. --- Lib/multiprocessing/spawn.py | 4 +++- .../Library/2023-07-05-13-08-23.gh-issue-90876.Qvlkfl.rst | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2023-07-05-13-08-23.gh-issue-90876.Qvlkfl.rst diff --git a/Lib/multiprocessing/spawn.py b/Lib/multiprocessing/spawn.py index 09f8a229d7cccb..5bf94a059fafaa 100644 --- a/Lib/multiprocessing/spawn.py +++ b/Lib/multiprocessing/spawn.py @@ -35,7 +35,9 @@ def set_executable(exe): global _python_exe - if sys.platform == 'win32': + if exe is None: + _python_exe = exe + elif sys.platform == 'win32': _python_exe = os.fsdecode(exe) else: _python_exe = os.fsencode(exe) diff --git a/Misc/NEWS.d/next/Library/2023-07-05-13-08-23.gh-issue-90876.Qvlkfl.rst b/Misc/NEWS.d/next/Library/2023-07-05-13-08-23.gh-issue-90876.Qvlkfl.rst new file mode 100644 index 00000000000000..3e062b5add6d89 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-07-05-13-08-23.gh-issue-90876.Qvlkfl.rst @@ -0,0 +1,3 @@ +Prevent :mod:`multiprocessing.spawn` from failing to *import* in environments +where ``sys.executable`` is ``None``. This regressed in 3.11 with the addition +of support for path-like objects in multiprocessing. From ec14f6d1f25db94626a0a29fbf78f87e2369010c Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Wed, 5 Jul 2023 13:25:25 -0700 Subject: [PATCH 2/9] Add a regression test. --- Lib/test/_test_multiprocessing.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index c101fe980ceed5..92c589376b5f93 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5849,6 +5849,18 @@ def test__all__(self): support.check__all__(self, multiprocessing, extra=multiprocessing.__all__, not_exported=['SUBDEBUG', 'SUBWARNING']) + def test_spawn_set_executable_none(self): + # Regression test for a bug introduced in + # https://github.com/python/cpython/issues/90876 that caused an + # ImportError in multiprocessing when sys.executable (or sys.exec_prefix + # on Windows) was None. This can be true in embedded environments. + import multiprocessing.spawn + orig_exe = multiprocessing.spawn.get_executable() + try: + multiprocessing.spawn.set_executable(None) + finally: + multiprocessing.spawn.set_executable(orig_exe) + # # Mixins From f6f8d5910ba7b95b05f11165c6adda8b23916ed7 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Wed, 5 Jul 2023 15:13:46 -0700 Subject: [PATCH 3/9] Test the actual problem, not the implementation. --- Lib/test/_test_multiprocessing.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 92c589376b5f93..b895bff28b0d65 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -31,6 +31,7 @@ from test.support import hashlib_helper from test.support import import_helper from test.support import os_helper +from test.support import script_helper from test.support import socket_helper from test.support import threading_helper from test.support import warnings_helper @@ -5849,17 +5850,23 @@ def test__all__(self): support.check__all__(self, multiprocessing, extra=multiprocessing.__all__, not_exported=['SUBDEBUG', 'SUBWARNING']) - def test_spawn_set_executable_none(self): + def test_spawn_sys_executable_none_allows_import(self): # Regression test for a bug introduced in # https://github.com/python/cpython/issues/90876 that caused an - # ImportError in multiprocessing when sys.executable (or sys.exec_prefix - # on Windows) was None. This can be true in embedded environments. - import multiprocessing.spawn - orig_exe = multiprocessing.spawn.get_executable() - try: - multiprocessing.spawn.set_executable(None) - finally: - multiprocessing.spawn.set_executable(orig_exe) + # ImportError in multiprocessing when sys.executable was None. + # This can be true in embedded environments. + testfn = os_helper.TESTFN + self.addCleanup(os_helper.unlink, testfn) + with open(testfn, 'wb') as f: + f.write(b'''\ +import sys +sys.executable = None +assert "multiprocessing" not in sys.modules, "mp already imported!" +import multiprocessing +import multiprocessing.spawn # This should not fail\n''') + rc, _, err = script_helper.assert_python_ok(testfn) + self.assertEqual(rc, 0) + self.assertEqual(err, b'') # From 3fc698ed5a3a9cb0856c00927c377fab83196e4d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 12:23:47 -0700 Subject: [PATCH 4/9] simplify test, skip in non-spawn suites. --- Lib/test/_test_multiprocessing.py | 33 +++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index b895bff28b0d65..29addfa53a9dbe 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -13,6 +13,7 @@ import os import gc import errno +import functools import signal import array import socket @@ -172,6 +173,28 @@ def check_enough_semaphores(): "to run the test (required: %d)." % nsems_min) +def only_run_in_spawn_testsuite(reason): + """Returns a decorator: raises SkipTest when SM != spawn at test time. + + This can be useful to save overall Python test suite execution time. + "spawn" is the universal mode available on all platforms so this limits the + decorated test to only execute within test_multiprocessing_spawn. + + This would not be necessary if we refactored our test suite to split things + into other test files when they are not start method specific to be rerun + under all start methods. + """ + + def decorator(test_item): + @functools.wraps(test_item) + def spawn_check_wrapper(*args, **kwargs): + if (start_method := multiprocessing.get_start_method()) != "spawn": + raise unittest.SkipTest(f"{start_method=}, not 'spawn'; {reason}") + return spawn_check_wrapper + + return decorator + + # # Creates a wrapper for a function which records the time it takes to finish # @@ -5850,21 +5873,19 @@ def test__all__(self): support.check__all__(self, multiprocessing, extra=multiprocessing.__all__, not_exported=['SUBDEBUG', 'SUBWARNING']) + @only_run_in_spawn_testsuite("avoids redundant testing.") def test_spawn_sys_executable_none_allows_import(self): # Regression test for a bug introduced in # https://github.com/python/cpython/issues/90876 that caused an # ImportError in multiprocessing when sys.executable was None. # This can be true in embedded environments. - testfn = os_helper.TESTFN - self.addCleanup(os_helper.unlink, testfn) - with open(testfn, 'wb') as f: - f.write(b'''\ + rc, _, err = script_helper.assert_python_ok( + "-c", """\ import sys sys.executable = None assert "multiprocessing" not in sys.modules, "mp already imported!" import multiprocessing -import multiprocessing.spawn # This should not fail\n''') - rc, _, err = script_helper.assert_python_ok(testfn) +import multiprocessing.spawn # This should not fail\n""") self.assertEqual(rc, 0) self.assertEqual(err, b'') From b2e1c13ed9fa90f86eb3cdd985039c9ceadcabf1 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 12:34:57 -0700 Subject: [PATCH 5/9] avoid \, use if 1: modernize the nearby test. --- Lib/test/_test_multiprocessing.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 29addfa53a9dbe..0b2f1672b4bd6c 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5839,29 +5839,27 @@ def test_namespace(self): class TestNamedResource(unittest.TestCase): + @only_run_in_spawn_testsuite("spawn specific test.") def test_global_named_resource_spawn(self): # # gh-90549: Check that global named resources in main module # will not leak by a subprocess, in spawn context. # - 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 + test_source = """if 1: + 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(testfn) + if __name__ == '__main__': + p = ctx.Process(target=submain) + p.start() + p.join() + """ + rc, out, err = script_helper.assert_python_ok("-c", test_source) # on error, err = 'UserWarning: resource_tracker: There appear to # be 1 leaked semaphore objects to clean up at shutdown' self.assertEqual(err, b'') @@ -5879,8 +5877,8 @@ def test_spawn_sys_executable_none_allows_import(self): # https://github.com/python/cpython/issues/90876 that caused an # ImportError in multiprocessing when sys.executable was None. # This can be true in embedded environments. - rc, _, err = script_helper.assert_python_ok( - "-c", """\ + rc, out, err = script_helper.assert_python_ok( + "-c", """if 1: import sys sys.executable = None assert "multiprocessing" not in sys.modules, "mp already imported!" From 3b311de29139661a86c8de3258cddb9913f90428 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 12:37:15 -0700 Subject: [PATCH 6/9] fix the windows unguarded sys.executable use. --- Lib/multiprocessing/spawn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/multiprocessing/spawn.py b/Lib/multiprocessing/spawn.py index 5bf94a059fafaa..f1af7709104714 100644 --- a/Lib/multiprocessing/spawn.py +++ b/Lib/multiprocessing/spawn.py @@ -31,7 +31,7 @@ WINSERVICE = False else: WINEXE = getattr(sys, 'frozen', False) - WINSERVICE = sys.executable.lower().endswith("pythonservice.exe") + WINSERVICE = sys.executable and sys.executable.lower().endswith("pythonservice.exe") def set_executable(exe): global _python_exe From cbafcf4359f08d0d65457b0839b77589647386ae Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 12:45:00 -0700 Subject: [PATCH 7/9] nicer indentation. --- Lib/test/_test_multiprocessing.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 0b2f1672b4bd6c..82f88736353d4c 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5878,12 +5878,14 @@ def test_spawn_sys_executable_none_allows_import(self): # ImportError in multiprocessing when sys.executable was None. # This can be true in embedded environments. rc, out, err = script_helper.assert_python_ok( - "-c", """if 1: -import sys -sys.executable = None -assert "multiprocessing" not in sys.modules, "mp already imported!" -import multiprocessing -import multiprocessing.spawn # This should not fail\n""") + "-c", + """if 1: + import sys + sys.executable = None + assert "multiprocessing" not in sys.modules, "already imported!" + import multiprocessing + import multiprocessing.spawn # This should not fail""", + ) self.assertEqual(rc, 0) self.assertEqual(err, b'') From 71422547c85a2b372b5edccfb61041e66804e8b0 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 15:08:13 -0700 Subject: [PATCH 8/9] *test* our new decorator... revert errant test change --- Lib/test/_test_multiprocessing.py | 74 +++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 82f88736353d4c..8197ceb590e217 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -186,15 +186,42 @@ def only_run_in_spawn_testsuite(reason): """ def decorator(test_item): + @functools.wraps(test_item) def spawn_check_wrapper(*args, **kwargs): if (start_method := multiprocessing.get_start_method()) != "spawn": raise unittest.SkipTest(f"{start_method=}, not 'spawn'; {reason}") + return test_item(*args, **kwargs) + return spawn_check_wrapper return decorator +class TestInternalDecorators(unittest.TestCase): + """Logic within a test suite that could errantly skip tests? Test it!""" + + def test_only_run_in_spawn_testsuite(self): + try: + @only_run_in_spawn_testsuite("testing this decorator") + def return_four_if_spawn(): + return 4 + except Exception as err: + self.fail(f"expected decorated `def` not to raise; caught {err}") + + orig_start_method = multiprocessing.get_start_method(allow_none=True) + try: + multiprocessing.set_start_method("spawn", force=True) + self.assertEqual(return_four_if_spawn(), 4) + multiprocessing.set_start_method("forkserver", force=True) + with self.assertRaises(unittest.SkipTest) as ctx: + return_four_if_spawn() + self.assertIn("testing this decorator", str(ctx.exception)) + self.assertIn("start_method=", str(ctx.exception)) + finally: + multiprocessing.set_start_method(orig_start_method, force=True) + + # # Creates a wrapper for a function which records the time it takes to finish # @@ -5845,24 +5872,23 @@ def test_global_named_resource_spawn(self): # gh-90549: Check that global named resources in main module # will not leak by a subprocess, in spawn context. # - test_source = """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 = script_helper.assert_python_ok("-c", test_source) + 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') + global_resource = ctx.Semaphore() + def submain(): pass + if __name__ == '__main__': + p = ctx.Process(target=submain) + p.start() + p.join() + ''')) + rc, out, err = 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'') + self.assertFalse(err, msg=err.decode('utf-8')) class MiscTestCase(unittest.TestCase): @@ -5878,16 +5904,16 @@ def test_spawn_sys_executable_none_allows_import(self): # ImportError in multiprocessing when sys.executable was None. # This can be true in embedded environments. rc, out, err = script_helper.assert_python_ok( - "-c", - """if 1: - import sys - sys.executable = None - assert "multiprocessing" not in sys.modules, "already imported!" - import multiprocessing - import multiprocessing.spawn # This should not fail""", + "-c", + """if 1: + import sys + sys.executable = None + assert "multiprocessing" not in sys.modules, "already imported!" + import multiprocessing + import multiprocessing.spawn # This should not fail\n""", ) self.assertEqual(rc, 0) - self.assertEqual(err, b'') + self.assertFalse(err, msg=err.decode('utf-8')) # From ba6f9fc1e1217c11341be2c67b39c1acf8eeb5c7 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 6 Jul 2023 15:15:24 -0700 Subject: [PATCH 9/9] Only run test_only_run_in_spawn_testsuite on non-windows within test_multiprocessing_spawn. --- Lib/test/_test_multiprocessing.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 8197ceb590e217..c1f9487ae80511 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -201,7 +201,11 @@ def spawn_check_wrapper(*args, **kwargs): class TestInternalDecorators(unittest.TestCase): """Logic within a test suite that could errantly skip tests? Test it!""" + @unittest.skipIf(sys.platform == "win32", "test requires that fork exists.") def test_only_run_in_spawn_testsuite(self): + if multiprocessing.get_start_method() != "spawn": + raise unittest.SkipTest("only run in test_multiprocessing_spawn.") + try: @only_run_in_spawn_testsuite("testing this decorator") def return_four_if_spawn(): @@ -213,7 +217,7 @@ def return_four_if_spawn(): try: multiprocessing.set_start_method("spawn", force=True) self.assertEqual(return_four_if_spawn(), 4) - multiprocessing.set_start_method("forkserver", force=True) + multiprocessing.set_start_method("fork", force=True) with self.assertRaises(unittest.SkipTest) as ctx: return_four_if_spawn() self.assertIn("testing this decorator", str(ctx.exception))