Skip to content

vm: unique non-Copy handle for trampoline frames - #8762

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:trampoline-iframe-handle
Sep 21, 2026
Merged

vm: unique non-Copy handle for trampoline frames#8762
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:trampoline-iframe-handle

Conversation

@youknowone

@youknowone youknowone commented Sep 21, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

One of checkbox below must be checked.

  • I did not use AI to write the code of this patch.
  • This PR follows our AI policy

Summary

Follow-up to #8737. The trampoline stores frames while it switches to a callee, then re-borrows them. That cannot stay as a live &mut InterpreterFrame for the whole call, so the loop used raw *mut.

This wraps the pointer in TrampolineIFrame:

  • not Copy, so two handles cannot alias the same allocation
  • as_mut(&mut self) so exclusive access goes through the handle
  • run_trampoline takes &mut InterpreterFrame again and converts at the door with from_mut
  • from_ptr stays unsafe for datastack callees and claimed generator frames

Pointer validity (datastack LIFO, generator PyRef + running claim) is still a construction contract. This only encodes “one &mut per frame at a time”.

Assisted-by: Grok:4.6

Summary by CodeRabbit

  • Refactor
    • Improved internal interpreter frame handling to ensure exclusive mutable access during execution.
    • Updated tail calls and generator resumption to preserve frame ownership and cleanup behavior.
    • Maintained stack release ordering and exception-handling behavior.

@youknowone
Give trampoline frames a unique non-Copy handle
d22fddd 
run_trampoline takes &mut again; the loop holds TrampolineIFrame so
two &mut InterpreterFrame cannot alias the same allocation.

Assisted-by: Grok:4.6
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 634d8aa8-7312-4cae-a7a9-a859f364d349

📥 Commits

Reviewing files that changed from the base of the PR and between 8035209 and d22fddd.

📒 Files selected for processing (1)
  • crates/vm/src/vm/mod.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

The trampoline now uses non-Copy TrampolineIFrame handles instead of raw mutable frame pointers. The handles flow through generator resumption, tail calls, cleanup, suspended outcomes, and exception handling.

Changes

Trampoline frame ownership

Layer / File(s) Summary
Frame handle contract
crates/vm/src/vm/mod.rs		 TrampolineIFrame provides unique mutable frame access. Suspended and generator-entered states now store this handle.
Execution and call propagation
crates/vm/src/vm/mod.rs		 Entry, generator resumption, tail-call handling, callee execution, cleanup, and data-stack release use TrampolineIFrame.
Suspended outcome delivery
crates/vm/src/vm/mod.rs		 Yield, value delivery, generator-stop, and exception paths mutate suspended frames through the handle while retaining existing dispatch behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: shaharnaveh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a unique, non-Copy handle for trampoline frames.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@youknowone
youknowone merged commit a46627a into RustPython:main Sep 21, 2026
30 checks passed
@youknowone
youknowone deleted the trampoline-iframe-handle branch September 21, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone