GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100227: Lock Around Adding Global Audit Hooks by ericsnowcurrently · Pull Request #105515 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented Jun 8, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

The risk of a race with this state is relatively low, but we play it safe anyway.

  • Issue: Improve Interpreter Isolation #100227

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ericsnowcurrently added 4 commits June 8, 2023 09:22
@ericsnowcurrently
Factor out LOCKS_INIT().
f868923
@ericsnowcurrently
Factor out _PyRuntimeState.audit_hooks.
3f0dcb4
@ericsnowcurrently
Add a granular lock for global audit hooks state.
f01297a
@ericsnowcurrently
Fix usage of the audit hooks lock.
8b83d9a
@ericsnowcurrently ericsnowcurrently added skip news needs backport to 3.12 only security fixes labels Jun 8, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jun 8, 2023
Improve Interpreter Isolation #100227
Closed
@ericsnowcurrently
ericsnowcurrently requested a review from zooba June 8, 2023 18:07
@ericsnowcurrently
Merge branch 'main' into audit-hooks-state-granular-lock
2e59238
@zooba

zooba commented Jun 8, 2023

Copy link
Copy Markdown
Member

I'd be just as happy with documenting that you need to self-synchronize when using this function. It should only be being called by embedders anyway.

But this change looks okay. I'm not as familiar with our usual locking pattern though, so I'm trusting you're following that.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently

ericsnowcurrently commented Jun 8, 2023

Copy link
Copy Markdown
Member Author

But this change looks okay. I'm not as familiar with our usual locking pattern though, so I'm trusting you're following that.

FWIW, we didn't have one before per-interpreter GIL. I've tried to be consistent, though, as I've been inventing the pattern. 😄
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently
ericsnowcurrently removed the request for review from zooba June 8, 2023 18:32
@ericsnowcurrently
ericsnowcurrently marked this pull request as ready for review June 8, 2023 18:32
@bedevere-bot bedevere-bot added the awaiting core review label Jun 8, 2023
@ericsnowcurrently
ericsnowcurrently enabled auto-merge (squash) June 8, 2023 18:33
@ericsnowcurrently
ericsnowcurrently merged commit e822a67 into python:main Jun 8, 2023
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting core review label Jun 8, 2023
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Sorry, @ericsnowcurrently, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e822a676f1f3bef6c5413e9b856db481c08ac2a5 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned ericsnowcurrently Jun 8, 2023
@ericsnowcurrently
ericsnowcurrently deleted the audit-hooks-state-granular-lock branch June 8, 2023 18:39
@ericsnowcurrently ericsnowcurrently added needs backport to 3.12 only security fixes and removed needs backport to 3.12 only security fixes labels Jun 8, 2023
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Sorry @ericsnowcurrently, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker e822a676f1f3bef6c5413e9b856db481c08ac2a5 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently ericsnowcurrently added needs backport to 3.12 only security fixes and removed needs backport to 3.12 only security fixes labels Jun 8, 2023
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Sorry @ericsnowcurrently, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker e822a676f1f3bef6c5413e9b856db481c08ac2a5 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently ericsnowcurrently removed the needs backport to 3.12 only security fixes label Jun 8, 2023
@ericsnowcurrently ericsnowcurrently added the needs backport to 3.12 only security fixes label Jun 8, 2023
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Sorry @ericsnowcurrently, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker e822a676f1f3bef6c5413e9b856db481c08ac2a5 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request Jun 8, 2023
@ericsnowcurrently
pythongh-100227: Lock Around Adding Global Audit Hooks (pythongh-105515)
986572b 
The risk of a race with this state is relatively low, but we play it safe anyway.
@bedevere-bot

bedevere-bot commented Jun 8, 2023

Copy link
Copy Markdown

GH-105525 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jun 8, 2023
ericsnowcurrently added a commit that referenced this pull request Jun 8, 2023
@ericsnowcurrently
[3.12] gh-100227: Lock Around Adding Global Audit Hooks (gh-105515) (g…
b08ea9a 
…h-105525)

The risk of a race with this state is relatively low, but we play it safe anyway.
(cherry picked from commit e822a67)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ericsnowcurrently ericsnowcurrently

Labels

skip news

Projects

None yet

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.

4 participants

@ericsnowcurrently @zooba @miss-islington @bedevere-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.