Skip to content

gh-132969: Fix error/hang when shutdown(wait=False) and task exited abnormally #133222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
Jun 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
8ac1fcf
gh-132969 Fix exception/hang when shutdown(wait=False) and a task ex…
ogbiggles Apr 30, 2025
1774445
📜🤖 Added by blurb_it.
blurb-it[bot] Apr 30, 2025
0938eeb
gh-132969 Skip forkserver-dependent test on Windows
blurb-it[bot] Apr 30, 2025
dbc0ba8
Merge branch 'origin/main' of github.com:ogbiggles/cpython-fix-132969…
ogbiggles Apr 30, 2025
99ab321
gh-132969 Skip forkserver dependent tests on Windows
ogbiggles Apr 30, 2025
fe3650c
Merge branch 'main' into origin/main
ogbiggles Apr 30, 2025
40cefc2
Merge branch 'python:main' into origin/main
ogbiggles May 8, 2025
c177c91
gh-132969 Added more verbiage to the explanation.
ogbiggles May 9, 2025
2cd70b1
gh-132969 Add more verbiage explaining the change
ogbiggles May 9, 2025
57265b5
gh-132969 Make test helper methods into private class methods, use @…
ogbiggles May 9, 2025
d0c2bb0
Merge branch 'origin/main' of github.com:ogbiggles/cpython-fix-132969…
ogbiggles May 9, 2025
709d9fd
gh-132969 Fix Sphinx Lint error
ogbiggles May 9, 2025
1a8919b
gh-132969: make unittest for gh-132969 runnable on windows by swichin…
ogbiggles May 9, 2025
87eb623
gh-132969: Remove @unittest.skipIf annotations since spawn works on w…
ogbiggles May 9, 2025
ec2543a
gh-132969: Incorproate review suggestion to skip test when start_meth…
ogbiggles May 9, 2025
3b3721f
gh-132969: Remove import of multiprocessing and use self.get_context(…
ogbiggles May 9, 2025
7ef7872
gh-132969: use self.skipTest instead of unittest.SkipTest, shorten me…
ogbiggles May 9, 2025
efd0a1d
gh-132969: removed type annotations
ogbiggles May 9, 2025
b335d38
gh-132969: removed extraneous blank line
ogbiggles May 9, 2025
b060786
gh-132969: fix call to self.skipTest
ogbiggles May 9, 2025
c6457c3
gh-132969: Run gh-132969 tests only with "spawn" mp_context
ogbiggles May 12, 2025
2beb0d9
Merge branch 'main' into origin/main
ogbiggles May 12, 2025
65f30b0
gh-132969: Join executor_manager_thread after exercising shutdown(wai…
ogbiggles May 12, 2025
54ae7b8
gh-132969: fix typo in skip message
ogbiggles May 12, 2025
e132f5e
gh-132969: Tweak the news entry for grammar and clarity.
ogbiggles May 12, 2025
9908810
gh-132969: Run test with "spawn" on all platforms and "forkserver" if…
ogbiggles May 12, 2025
f5f3dc2
Update Lib/test/test_concurrent_futures/test_shutdown.py
ogbiggles Jun 2, 2025
0fe743b
Update Misc/NEWS.d/next/Library/2025-04-30-19-32-18.gh-issue-132969.E…
ogbiggles Jun 2, 2025
310765e
gh-132969: fixed typo in reference to shutdown method
ogbiggles Jun 2, 2025
86d0c2a
gh-132969: fixed doc reference to shutdown method
ogbiggles Jun 2, 2025
4f46875
gh-132969: Tweak comment for readability
ogbiggles Jun 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Lib/concurrent/futures/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,11 @@ def _start_executor_manager_thread(self):
self._executor_manager_thread_wakeup

def _adjust_process_count(self):
# gh-132969: avoid error when state is reset and executor is still running,
# which will happen when shutdown(wait=False) is called.
if self._processes is None:
return

# if there's an idle process, we don't need to spawn a new one.
if self._idle_worker_semaphore.acquire(blocking=False):
return
Expand Down
58 changes: 58 additions & 0 deletions Lib/test/test_concurrent_futures/test_shutdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,64 @@ def test_shutdown_no_wait(self):
# shutdown.
assert all([r == abs(v) for r, v in zip(res, range(-5, 5))])

@classmethod
def _failing_task_gh_132969(cls, n):
raise ValueError("failing task")

@classmethod
def _good_task_gh_132969(cls, n):
time.sleep(0.1 * n)
return n

def _run_test_issue_gh_132969(self, max_workers):
# max_workers=2 will repro exception
# max_workers=4 will repro exception and then hang

# Repro conditions
# max_tasks_per_child=1
# a task ends abnormally
# shutdown(wait=False) is called
start_method = self.get_context().get_start_method()
if (start_method == "fork" or
(start_method == "forkserver" and sys.platform.startswith("win"))):
self.skipTest(f"Skipping test for {start_method = }")
executor = futures.ProcessPoolExecutor(
max_workers=max_workers,
max_tasks_per_child=1,
mp_context=self.get_context())
f1 = executor.submit(ProcessPoolShutdownTest._good_task_gh_132969, 1)
f2 = executor.submit(ProcessPoolShutdownTest._failing_task_gh_132969, 2)
f3 = executor.submit(ProcessPoolShutdownTest._good_task_gh_132969, 3)
result = 0
try:
result += f1.result()
result += f2.result()
result += f3.result()
except ValueError:
# stop processing results upon first exception
pass

# Ensure that the executor cleans up after called
# shutdown with wait=False
executor_manager_thread = executor._executor_manager_thread
executor.shutdown(wait=False)
time.sleep(0.2)
executor_manager_thread.join()
return result

def test_shutdown_gh_132969_case_1(self):
# gh-132969: test that exception "object of type 'NoneType' has no len()"
# is not raised when shutdown(wait=False) is called.
result = self._run_test_issue_gh_132969(2)
self.assertEqual(result, 1)

def test_shutdown_gh_132969_case_2(self):
# gh-132969: test that process does not hang and
# exception "object of type 'NoneType' has no len()" is not raised
# when shutdown(wait=False) is called.
result = self._run_test_issue_gh_132969(4)
self.assertEqual(result, 1)


create_executor_tests(globals(), ProcessPoolShutdownTest,
executor_mixins=(ProcessPoolForkMixin,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Prevent the :class:`~concurrent.futures.ProcessPoolExecutor` executor thread,
which remains running when :meth:`shutdown(wait=False)
<concurrent.futures.Executor.shutdown>`, from
attempting to adjust the pool's worker processes after the object state has already been reset during shutdown.
A combination of conditions, including a worker process having terminated abormally,
resulted in an exception and a potential hang when the still-running executor thread
attempted to replace dead workers within the pool.
Loading