Skip to content

gh-136396: Include instrumentation when creating new copies of the bytecode #136525

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

Merged
merged 3 commits into from
Jul 14, 2025

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Jul 10, 2025

Previously, we assumed that instrumentation would happen for all copies of the bytecode if the instrumentation version on the code object didn't match the per-interpreter instrumentation version. That assumption was incorrect: instrumentation will exit early if there are no new "events," even if there is an instrumentation version mismatch.

To fix this, include the instrumented opcodes when creating new copies of the bytecode, rather than replacing them with their uninstrumented variants. I don't think we have to worry about races between instrumentation and creating new copies of the bytecode as they cannot happen concurrently. Instrumentation requires that either the world is stopped or the code object's per-object lock is held, while new bytecode creation requires holding the code object's per-object lock.

Previously, we assumed that instrumentation would happen for all copies of
the bytecode if the instrumentation version on the code object didn't match
the per-interpreter instrumentation version. That assumption was incorrect:
instrumentation will exit early if there are no new "events," even if there
is an instrumentation version mismatch.

To fix this, include the instrumented opcodes when creating new copies of
the bytecode, rather than replacing them with their uninstrumented variants.
I don't think we have to worry about races between instrumentation and creating
new copies of the bytecode: instrumentation and new bytecode creation cannot happen
concurrently. Instrumentation requires that either the world is stopped or the
code object's per-object lock is held and new bytecode creation requires holding
the code object's per-object lock.
@mpage mpage marked this pull request as ready for review July 10, 2025 23:24
@mpage mpage requested a review from markshannon as a code owner July 10, 2025 23:24
@mpage mpage requested review from colesbury and Yhg1s July 10, 2025 23:24
@mpage mpage merged commit d995922 into python:main Jul 14, 2025
41 checks passed
@mpage mpage added the needs backport to 3.14 bugs and security fixes label Jul 14, 2025
@miss-islington-app
Copy link

Thanks @mpage for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 14, 2025
…the bytecode (pythonGH-136525)

Previously, we assumed that instrumentation would happen for all copies of
the bytecode if the instrumentation version on the code object didn't match
the per-interpreter instrumentation version. That assumption was incorrect:
instrumentation will exit early if there are no new "events," even if there
is an instrumentation version mismatch.

To fix this, include the instrumented opcodes when creating new copies of
the bytecode, rather than replacing them with their uninstrumented variants.
I don't think we have to worry about races between instrumentation and creating
new copies of the bytecode: instrumentation and new bytecode creation cannot happen
concurrently. Instrumentation requires that either the world is stopped or the
code object's per-object lock is held and new bytecode creation requires holding
the code object's per-object lock.
(cherry picked from commit d995922)

Co-authored-by: mpage <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 14, 2025

GH-136657 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants