Skip to content
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

Jump to bottom

gh-99377: Revert audit events for thread state creation and free, because the GIL is not properly held at these times #99543

Merged
merged 3 commits into from Nov 17, 2022
Merged

gh-99377: Revert audit events for thread state creation and free, because the GIL is not properly held at these times #99543

merged 3 commits into from Nov 17, 2022
+1 −39

Conversation

zooba
Copy link
Member

@zooba zooba commented Nov 16, 2022 •
edited by bedevere-bot

This can occur during initialization. Without the check, later calls to get the interpreter state will cause a fatal error.

@zooba
pythongh-99377: Prevent raising audit event when thread state has not…
c732ecc 
… actually been set.

This can occur during initialization. Without the check, later calls to get the interpreter state will cause a fatal error.
@zooba zooba requested a review from ericsnowcurrently Nov 16, 2022
@bedevere-bot bedevere-bot mentioned this pull request Nov 16, 2022
Open
@zooba zooba added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Nov 16, 2022
@bedevere-bot
Copy link

bedevere-bot commented Nov 16, 2022

🤖 New build scheduled with the buildbot fleet by @zooba for commit c732ecc 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 16, 2022
@zooba zooba mentioned this pull request Nov 16, 2022
Merged
ericsnowcurrently
ericsnowcurrently approved these changes Nov 16, 2022
View changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

LGTM

I'm surprised _PyThreadState_SetCurrent() can leave it NULL though. Something seems fishy there.

@zooba
Copy link
Member Author

zooba commented Nov 16, 2022

This hasn't done it. Going to revert most of the change (I'll leave the _thread event in there, just not the ThreadState ones)

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Nov 16, 2022

FYI, I'm double-checking that that PR was the culprit: gh-99545.

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Nov 16, 2022

Looks like a full revert passes all buildbots (actually, 7 still running), so it seems like we're right that the original PR is the source of the failures.

@ericsnowcurrently ericsnowcurrently mentioned this pull request Nov 16, 2022
Closed
ericsnowcurrently
ericsnowcurrently approved these changes Nov 16, 2022
View changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

LGTM

@sobolevn
Copy link
Member

sobolevn commented Nov 16, 2022

This also closes #99544

@zooba zooba changed the title gh-99377: Prevent raising audit event when thread state has not actually been set. gh-99377: Revert audit events for thread state creation and free, because the GIL is not properly held at these times Nov 16, 2022
@AlexWaygood AlexWaygood linked an issue Nov 16, 2022 that may be closed by this pull request
Buildbots are crashing after #99378 #99544
Closed
@zooba zooba merged commit 5fdd49d into python:main Nov 17, 2022
15 checks passed
@zooba zooba deleted the gh-99377 branch Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericsnowcurrently ericsnowcurrently

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Buildbots are crashing after #99378
4 participants
@zooba @bedevere-bot @ericsnowcurrently @sobolevn