GeezerVison is showing github.com — bpo-44422: threading.Thread reuses the _delete() method by vstinner · Pull Request #26741 · python/cpython
Conversation
vstinner
commented
Jun 15, 2021
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
The _bootstrap_inner() method of threading.Thread now reuses its
_delete() method rather than accessing _active() directly. It became
possible since _active_limbo_lock became reentrant. Moreover, it no
longer ignores any exception when deleting the thread from the
_active dictionary.
https://bugs.python.org/issue44422
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The _bootstrap_inner() method of threading.Thread now reuses its
_delete() method rather than accessing _active() directly. It became
possible since _active_limbo_lock became reentrant. Moreover, it no
longer ignores any exception when deleting the thread from the
_active dictionary.
|
History:
- (B) commit 95cd5c0 replaces the
_delete() call with inlined code (modify directly _active dictionary)
- Fix Issue #1703448: A joined thread could show up in the
threading.enumerate() list after the join() for a brief period until
it actually exited.
- (A) commit f21b2aa added
try: ... except: pass
Thread.__bootstrap(): ignore exceptions in the self.__delete() call in
the finally clause. An exception here could happen when a daemon
thread exits after the threading module has already been trashed by
the import finalization, and there's not much of a point in trying to
insist doing the cleanup in that stage.
This should fix SF bug ##497111: active_limbo_lock error at program
exit.
2.1.2 and 2.2.1 Bugfix candidate!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
(B) commit 95cd5c0 replaces the _delete() call with inlined code (modify directly _active dictionary)
This is https://bugs.python.org/issue1703448
The fix adds test_enumerate_after_join() to Lib/test/test_threading.py which still exists. Running test_threading should be enough to check for non-regression.
(A) commit f21b2aa added try: ... except: pass
This is https://bugs.python.org/issue497111
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
How did you conclude that the try-except is no longer needed?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
How did you conclude that the try-except is no longer needed?
except: pass ignores any exception. At least, we should only catch KeyError.
I don't understand in which case an exception can occur. In case of doubt, I prefer to remove it. We can add the try/except KeyError again if someone hits the issue.
https://bugs.python.org/issue497111 was reported 20 years ago. The Python code base changed a lot in 20 years.
For example, I modified the Python finalization to clear daemon threads earlier: commit 9ad58ac.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
🤖 New build scheduled with the buildbot fleet by @vstinner for commit d32b7fc 🤖
If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
jdevries3133
pushed a commit
to jdevries3133/cpython
that referenced
this pull request
Jun 19, 2021
The _bootstrap_inner() method of threading.Thread now reuses its
_delete() method rather than accessing _active() directly. It became
possible since _active_limbo_lock became reentrant. Moreover, it no
longer ignores any exception when deleting the thread from the
_active dictionary.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|