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 — Create C API worker VMs without attaching the template by youknowdot · Pull Request #9318 · RustPython/RustPython

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Create the C API worker VM through Interpreter::new_thread() instead of temporarily entering the template VM while holding MAIN_INTERP. The existing attachment path still attaches the returned worker after the factory returns and releases that mutex.

On current main, both calls use the same interpreter receiver and clone implementation, preserving the worker's owned shared state. The old path can park during attachment while retaining MAIN_INTERP, including shutdown parking that can prevent later registration cleanup from acquiring the mutex. This is a source-supported possible interleaving, not a runtime-reproduced shutdown race.

This change removes that attachment-under-mutex edge. It does not remove the existing trace/profile locks or establish general callback, admission, retirement, or finalization safety.

Regression test and validation

Add one native structural test using a fresh OS thread. Merely constructing a worker must leave both the VM stack and current thread slot empty. The parent detaches while joining, then drops the returned clone under its attached interpreter before assertions. No shutdown, stop-the-world, race, or callback probe is performed.

On Linux x86_64:

  • The identical test on the original factory failed exactly the final cached-slot assertion: (false, true) instead of (false, false).
  • With this change, the exact test passed: 1 passed, 0 failed, 0 ignored, with 130 other tests filtered out.
  • Built the lib-test using cargo test --locked --offline -j1 -p rustpython-capi --lib --no-run, from crates/capi, and executed only pylifecycle::tests::requesting_thread_vm_does_not_attach from separately preserved baseline/candidate executables. Neither links host libpython.
  • cargo clippy --locked --offline -j1 -p rustpython-capi --lib --tests -- -D warnings passed, also from crates/capi.
  • Normal commit hooks passed. The full C API suite was not run.

The tested base is 0231d847d9da0eed90c3fe87010205683d52af56. Main subsequently advanced to f65bf8e52d0c0a387bca897614befa1aa4af9d69 with a Cargo.lock-only dependency update. Source-only merge composition is clean and preserves this exact C API file; that newer-main combination was not executed locally.

Upgrade integration

This deliberately extracts a current-main prerequisite overlapping #8944, rather than claiming independent ownership. The same caller replacement also appears in the broader GC redesign #8939. #8944 already has the same caller replacement, but its Interpreter::new_thread() enters through enter_raw. This test is an explicit integration gate: that owner branch must preserve the factory's no-attachment boundary even when the textual caller change merges cleanly.

This PR does not use its restricted idle-anchor API, change its owner/admission design, or modify #8944.

AI assistance was used in source review, implementation, and validation.

Sorry, something went wrong.

Uh oh!

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

chatgpt-codex-connector[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@codex
Avoid attaching C API worker templates under the interpreter lock
adecad6
Keep worker VM cloning separate from attachment and add a native structural regression test.

Assisted-by: OpenAI Codex
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Source mode: Auto
  • Repository: RustPython/RustPython
  • Branch: main
  • File: .coderabbit.yml
  • Commit: f65bf8e
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1b8ec0df-bbe8-41ce-b38e-696c712fad66
📥 Commits

Reviewing files that changed from the base of the PR and between f65bf8e and adecad6.

📒 Files selected for processing: 1
  • crates/capi/src/pylifecycle.rs
+---------------------------------------------+
| Preventing Skynet from becoming self-aware. |
+---------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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

All reactions

Sorry, something went wrong.

Uh oh!

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

youknowone
youknowone approved these changes Oct 23, 2026
View reviewed changes
@youknowone
youknowone marked this pull request as ready for review October 23, 2026 08:57
@youknowone
youknowone merged commit ec299f3 into RustPython:main Oct 23, 2026
21 checks passed

Uh oh!

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T09:01:27.833628Z adecad6 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

3 participants

@youknowdot @youknowone @codex
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.