Skip to content

gh-131788: make resource_tracker re-entrant safe #131787

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Aug 13, 2025
Merged

gh-131788: make resource_tracker re-entrant safe #131787

merged 18 commits into from
Aug 13, 2025

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Mar 27, 2025 •
edited by bedevere-app bot
Loading

@graingert graingert force-pushed the make-resource-tracker-reentrant-safe branch 3 times, most recently from cecd4e3 to b99fcbe Compare March 27, 2025 07:19
@graingert graingert force-pushed the make-resource-tracker-reentrant-safe branch from b99fcbe to 9114b16 Compare March 27, 2025 07:23
@graingert graingert changed the title make resource_tracker re-entrant safe gh-131788: make resource_tracker re-entrant safe Mar 27, 2025
@bedevere-app bedevere-app bot mentioned this pull request Mar 27, 2025
Open
@graingert
Copy link
Contributor Author

see also https://github.com/python/cpython/pull/109629/files

@graingert graingert marked this pull request as ready for review March 27, 2025 08:45
@graingert graingert requested a review from gpshead as a code owner March 27, 2025 08:45
@graingert graingert closed this Mar 27, 2025
@graingert graingert reopened this Mar 27, 2025
gpshead
gpshead reviewed Mar 29, 2025
View reviewed changes
@graingert graingert requested a review from gpshead March 29, 2025 10:32
gpshead
gpshead reviewed Mar 29, 2025
View reviewed changes
@gpshead
Copy link
Member

gpshead commented Mar 29, 2025 •
edited
Loading

FWIW I'm not sure we should backport this to stable releases as a bug fix. Unless someone is seeing it in their actual application code, lets keep the change risk low?

Or conservatively hold off on backporting until this gets some exposure in a few 3.14 (beta) releases.

@graingert graingert requested a review from gpshead May 4, 2025 07:37
@graingert
Copy link
Contributor Author

It will need to be merged soon to get it into the release!

@gpshead gpshead enabled auto-merge (squash) May 5, 2025 01:05
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025
@ZeroIntensity ZeroIntensity removed the needs backport to 3.12 only security fixes label May 29, 2025
@colesbury
Copy link
Contributor

I'm seeing the re-entrant error frequently in the CI and would love to see this fixed. I'm not sure what test failures were on this PR because they got deleted due to age, so I merged main back into the PR branch.

@gpshead gpshead merged commit f24a012 into python:main Aug 13, 2025
44 checks passed
@miss-islington-app
Copy link

Thanks @graingert for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 13, 2025
@graingert @blurb-it
@gpshead @miss-islington
pythongh-131788: make resource_tracker re-entrant safe (pythonGH-131787)
889aaab 
* make resource_tracker re-entrant safe
* Update Lib/multiprocessing/resource_tracker.py
* trim trailing whitespace
* use f-string and args = [x, *y, z]
* raise self._reentrant_call_error

---------
(cherry picked from commit f24a012)

Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 13, 2025
@graingert @blurb-it
@gpshead @miss-islington
pythongh-131788: make resource_tracker re-entrant safe (pythonGH-131787)
690fc81 
* make resource_tracker re-entrant safe
* Update Lib/multiprocessing/resource_tracker.py
* trim trailing whitespace
* use f-string and args = [x, *y, z]
* raise self._reentrant_call_error

---------
(cherry picked from commit f24a012)

Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
@bedevere-app
Copy link

bedevere-app bot commented Aug 13, 2025

GH-137737 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Aug 13, 2025
@bedevere-app
Copy link

bedevere-app bot commented Aug 13, 2025

GH-137738 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 13, 2025
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 CentOS9 NoGIL Refleaks 3.x (tier-1) has failed when building commit f24a012.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1610/builds/1903) and take a look at the build logs.
  4. Check if the failure is related to this commit (f24a012) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1610/builds/1903

Failed tests:

  • test_free_threading

Test leaking resources:

  • test_free_threading: memory blocks
  • test_free_threading: file descriptors

Summary of the results of the build (if available):

==

Click to see traceback logs 
remote: Enumerating objects: 10, done.        
remote: Counting objects:  11% (1/9)        
remote: Counting objects:  22% (2/9)        
remote: Counting objects:  33% (3/9)        
remote: Counting objects:  44% (4/9)        
remote: Counting objects:  55% (5/9)        
remote: Counting objects:  66% (6/9)        
remote: Counting objects:  77% (7/9)        
remote: Counting objects:  88% (8/9)        
remote: Counting objects: 100% (9/9)        
remote: Counting objects: 100% (9/9), done.        
remote: Compressing objects:  33% (1/3)        
remote: Compressing objects:  66% (2/3)        
remote: Compressing objects: 100% (3/3)        
remote: Compressing objects: 100% (3/3), done.        
remote: Total 10 (delta 6), reused 7 (delta 6), pack-reused 1 (from 1)        
From https://github.com/python/cpython
 * branch                    main       -> FETCH_HEAD
Note: switching to 'f24a012350f71141648cbd61081a25a458dd7fff'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at f24a012350f gh-131788: make resource_tracker re-entrant safe (GH-131787)
Switched to and reset branch 'main'

configure: WARNING: no system libmpdec found; falling back to pure-Python version for the decimal module

make: *** [Makefile:2486: buildbottest] Error 2

Yhg1s pushed a commit that referenced this pull request Aug 14, 2025
@miss-islington @graingert
@blurb-it @gpshead
[3.13] gh-131788: make resource_tracker re-entrant safe (GH-131787) (#…
d22276d 
…137738)

gh-131788: make resource_tracker re-entrant safe (GH-131787)
(cherry picked from commit f24a012)

Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
@lesteve lesteve mentioned this pull request Aug 19, 2025
Closed
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@graingert @blurb-it
@gpshead @Agent-Hellboy
pythongh-131788: make resource_tracker re-entrant safe (pythonGH-131787)
dc132c0 
* make resource_tracker re-entrant safe
* Update Lib/multiprocessing/resource_tracker.py
* trim trailing whitespace
* use f-string and args = [x, *y, z]
* raise self._reentrant_call_error

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
tomMoral
tomMoral reviewed Aug 26, 2025
View reviewed changes
Copy link
Contributor

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
While reading the code to adapt to this change in joblib/loky#461, I think there is some dead code that could be cleaned up.

Lib/multiprocessing/resource_tracker.py
break
self._write(reentrant_msg)
if msg is not None:
self._write(msg)

def _check_alive(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is not called anymore, the method can be clean up.

zanieb added a commit to zanieb/cpython that referenced this pull request Aug 28, 2025
@zanieb
Revert "[3.13] pythongh-131788: make resource_tracker re-entrant safe (…
7e48061 
…pythonGH-131787) (python#137738)"

This reverts commit d22276d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead gpshead approved these changes

+1 more reviewer

@tomMoral tomMoral tomMoral left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@graingert @gpshead @colesbury @bedevere-bot @tomMoral @serhiy-storchaka @ZeroIntensity