Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-104090: Update Resource Tracker to return exit code based on resource leaked found or not#106807
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
Uh oh!
There was an error while loading. Please reload this page.
gh-104090: Update Resource Tracker to return exit code based on resource leaked found or not #106807
Changes from all commits
70a35a2da7e2451de022d48bea8431bc4af47551a3a02fdbe9d0369e4dbb777bfd1e8e043ae90850b5bb6e82940a5cb934ccd736c3b21d5cfbd18ece258cb35b886bf915c8a3cd377e627245ecFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -61,6 +61,7 @@ def __init__(self): | ||
| self._lock = threading.RLock() | ||
| self._fd = None | ||
| self._pid = None | ||
| self._exitcode = None | ||
| def _reentrant_call_error(self): | ||
| # gh-109629: this happens if an explicit call to the ResourceTracker | ||
| @@ -84,9 +85,16 @@ def _stop(self): | ||
| os.close(self._fd) | ||
| self._fd = None | ||
| os.waitpid(self._pid, 0) | ||
| _, status = os.waitpid(self._pid, 0) | ||
| self._pid = None | ||
| try: | ||
| self._exitcode = os.waitstatus_to_exitcode(status) | ||
| except ValueError: | ||
| # os.waitstatus_to_exitcode may raise an exception for invalid values | ||
| self._exitcode = None | ||
| def getfd(self): | ||
| self.ensure_running() | ||
| return self._fd | ||
| @@ -119,6 +127,7 @@ def ensure_running(self): | ||
| pass | ||
| self._fd = None | ||
| self._pid = None | ||
| self._exitcode = None | ||
| warnings.warn('resource_tracker: process died unexpectedly, ' | ||
| 'relaunching. Some resources might leak.') | ||
| @@ -221,6 +230,8 @@ def main(fd): | ||
| pass | ||
| cache ={rtype: set() for rtype in _CLEANUP_FUNCS.keys()} | ||
| exit_code = 0 | ||
| try: | ||
| # keep track of registered/unregistered resources | ||
| with open(fd, 'rb') as f: | ||
| @@ -251,6 +262,7 @@ def main(fd): | ||
| for rtype, rtype_cache in cache.items(): | ||
| if rtype_cache: | ||
| try: | ||
| exit_code = 1 | ||
| warnings.warn( | ||
| f'resource_tracker: There appear to be{len(rtype_cache)} ' | ||
| f'leaked{rtype} objects to clean up at shutdown:{rtype_cache}' | ||
| @@ -268,3 +280,5 @@ def main(fd): | ||
| warnings.warn('resource_tracker: %r: %s' % (name, e)) | ||
| finally: | ||
| pass | ||
| sys.exit(exit_code) | ||
iritkatriel marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -5730,6 +5730,47 @@ def test_too_long_name_resource(self): | ||
| with self.assertRaises(ValueError): | ||
| resource_tracker.register(too_long_name_resource, rtype) | ||
| def _test_resource_tracker_leak_resources(self, context, delete_queue): | ||
| from multiprocessing.resource_tracker import _resource_tracker | ||
| _resource_tracker.ensure_running() | ||
| self.assertTrue(_resource_tracker._check_alive()) | ||
| # Reset exit code value | ||
| _resource_tracker._exitcode = None | ||
| mp_context = multiprocessing.get_context(context) | ||
| # Keep it on variable, so it won't be cleared yet | ||
| q = mp_context.Queue() | ||
| if delete_queue: | ||
| # Clearing the queue resource to be sure explicitly with deleting | ||
| # and gc.collect | ||
| q.close() | ||
| del q | ||
sunmy2019 marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| gc.collect() | ||
| expected_exit_code = 0 | ||
| else: | ||
| expected_exit_code = 1 | ||
| self.assertIsNone(_resource_tracker._exitcode) | ||
| _resource_tracker._stop() | ||
| self.assertEqual(_resource_tracker._exitcode, expected_exit_code) | ||
| def test_resource_tracker_exit_code(self): | ||
sunmy2019 marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| """ | ||
| Test the exit code of the resource tracker based on if there were left | ||
| leaked resources when we stop the process. If not leaked resources were | ||
| found, exit code should be 0, otherwise 1 | ||
| """ | ||
| for context in ["spawn", "forkserver"]: | ||
pitrou marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| for delete_queue in [True, False]: | ||
| with self.subTest(context=context, delete_queue=delete_queue): | ||
| self._test_resource_tracker_leak_resources( | ||
| context=context, | ||
| delete_queue=delete_queue, | ||
| ) | ||
| class TestSimpleQueue(unittest.TestCase): | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| The multiprocessing resource tracker now exits with status code 1 if a resource | ||
| leak was detected. It still exits with status code 0 otherwise. |
Uh oh!
There was an error while loading. Please reload this page.