Skip to content

gh-88887: Cleanup multiprocessing.resource_tracker.ResourceTracker upon deletion #130429

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

Merged
merged 21 commits into from
Mar 20, 2025
Merged

gh-88887: Cleanup multiprocessing.resource_tracker.ResourceTracker upon deletion #130429

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
069fb0c
fix resource tracker leak
luccabb Feb 21, 2025
5e33828
try block
luccabb Feb 21, 2025
0586696
add comment to resource tracker __del__
luccabb Feb 21, 2025
eeef2e4
blurb add
luccabb Feb 21, 2025
b3115d2
Merge branch 'main' into fix_resource_tracker_leak
luccabb Feb 21, 2025
cf3d546
Merge branch 'main' of github.com:luccabb/cpython into fix_resource_t…
luccabb Feb 24, 2025
933f70c
catch specific exceptions
luccabb Feb 24, 2025
d4e08c5
Merge branch 'fix_resource_tracker_leak' of github.com:luccabb/cpytho…
luccabb Feb 24, 2025
799ee32
Update Lib/multiprocessing/resource_tracker.py
luccabb Feb 24, 2025
8e43a83
fixing _stop for None pids
luccabb Mar 6, 2025
f9a53a7
pull
luccabb Mar 6, 2025
805ad1d
remove :77 typo
luccabb Mar 6, 2025
5d6c9b3
add extra line back
luccabb Mar 6, 2025
c8e2704
catch AttributeError only and return early from _stop if _pid is None
luccabb Mar 6, 2025
9fd5872
fix lint
luccabb Mar 6, 2025
aaa8378
injecting os methods on _stop
luccabb Mar 7, 2025
dc2d578
Update Lib/multiprocessing/resource_tracker.py comment
gpshead Mar 7, 2025
9847342
non blocking acquire for __del__
luccabb Mar 9, 2025
58b137a
non blocking acquire for __del__
luccabb Mar 9, 2025
d497dee
release only if acquired
luccabb Mar 10, 2025
ba42406
always release + changelog
luccabb Mar 19, 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
9 changes: 9 additions & 0 deletions Lib/multiprocessing/resource_tracker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,15 @@ def _send(self, cmd, name, rtype):
assert nbytes == len(msg), "nbytes {0:n} but len(msg) {1:n}".format(
nbytes, len(msg))

def __del__(self):
# making sure child processess are cleaned before ResourceTracker
# gets destructed.
# see https://github.com/python/cpython/issues/88887
try:
self._stop()
except (OSError, TypeError, AttributeError):
Copy link
Member

Choose a reason for hiding this comment

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

I'm not confortable with these exceptions. In which case we should expect these exceptions? I would prefer a direct call to self._stop() without try/except.

_stop() should be modified to do nothing if self._pid is None.

    def _stop(self):
        with self._lock:
            if self._pid is None:
                return
            ...

Copy link
Contributor Author

@luccabb luccabb Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

moved to catching AttributeError only which is likely caused by __del__:

del() can be executed during interpreter shutdown. As a consequence, the global variables it needs to access (including other modules) may already have been deleted or set to None.

https://docs.python.org/3/reference/datamodel.html#object.del

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not catching AttributeError here fails some of the tests as the os module is already None when _stop runs

pass


_resource_tracker = ResourceTracker()
ensure_running = _resource_tracker.ensure_running
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-14-47-46.gh-issue-88887.V3U0CV.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixing multiprocessing Resource Tracker process leaking
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Fixing multiprocessing Resource Tracker process leaking
Fixing multiprocessing Resource Tracker process leaking.

Copy link
Member

Choose a reason for hiding this comment

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

Is the issue specific to Python running as the pid 1? If yes, it might be useful to mention it in the Changelog entry.

Copy link
Contributor Author

@luccabb luccabb Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the leaking happens regardless of PID 1. Python relies on kernel's process handling code to clean up the process, which works fine for most cases.

A case where it won't work is when running Python as PID 1 and there's no signal processing handler to reap zombies on client code

Loading