Skip to content

[3.12] gh-110206: Fix multiprocessing test_notify_all (GH-130933) #130951

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 1 commit into from
Mar 7, 2025

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 7, 2025

The test could deadlock trying join on the worker processes due to a
combination of behaviors:

  • The use of assertReachesEventually did not ensure that workers
    actually woken.release() because the SyncManager's Semaphore does not
    implement get_value.

  • This mean that the test could finish and the variable "sleeping" would
    got out of scope and be collected. This unregisters the proxy leading
    to failures in the worker or possibly the manager.

  • The subsequent call to p.join() during cleanUp therefore never
    finished.

This takes two approaches to fix this:

  1. Use woken.acquire() to ensure that the workers actually finish
    calling woken.release()

  2. At the end of the test, wait until the workers are finished, while cond,
    sleeping, and woken are still valid.
    (cherry picked from commit c476410)

Co-authored-by: Sam Gross [email protected]

The test could deadlock trying join on the worker processes due to a
combination of behaviors:

* The use of `assertReachesEventually` did not ensure that workers
  actually woken.release() because the SyncManager's Semaphore does not
  implement get_value.

* This mean that the test could finish and the variable "sleeping" would
  got out of scope and be collected. This unregisters the proxy leading
  to failures in the worker or possibly the manager.

* The subsequent call to `p.join()` during cleanUp therefore never
  finished.

This takes two approaches to fix this:

1) Use woken.acquire() to ensure that the workers actually finish
   calling woken.release()

2) At the end of the test, wait until the workers are finished, while `cond`,
   `sleeping`, and `woken` are still valid.
(cherry picked from commit c476410)

Co-authored-by: Sam Gross <[email protected]>
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 7f85038 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130951%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 7, 2025
@colesbury colesbury merged commit 4230697 into python:3.12 Mar 7, 2025
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants