Skip to content

Isolate GC ownership test from child automatic collection - #9196

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/gc-ownership-test-isolation
Oct 19, 2026
Merged

Isolate GC ownership test from child automatic collection#9196
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/gc-ownership-test-isolation

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Disable automatic GC in the child interpreter before constructing the cycle in collections_only_reach_the_collecting_interpreter.

This test verifies that a forced collection in the parent does not collect objects owned by the child. The child's automatic collector can otherwise collect its own unreachable cycle before the first live_nodes assertion. Disable that independent collection path while keeping both live_nodes == 2 assertions, the cycle itself, and the parent's forced-collection retry unchanged.

The source supports this explanation for the intermittent pre-collection assertion failure reported during #9185. That original failure has not been independently reproduced here.

Validation

  • Targeted threaded GC ownership test: passed, plus 25 consecutive repetitions.
  • Full threaded VM tests: 152 passed; doctests 17 passed, 2 existing ignored.
  • cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: passed (1,400 passed, 18 existing ignored across tests and doctests).
  • (cd crates/capi && cargo test): 123 passed, 4 existing ignored.
  • cargo clippy --workspace --all-targets --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi -- -D warnings: passed.
  • (cd crates/capi && cargo clippy --all-targets -- -D warnings): passed.
  • Normal commit hooks: passed, including rustfmt and cspell.
  • git diff --check: passed.

Validated using Rust 1.99.1. An initial link attempt lacked the libffi development library; retry and all gates above passed after adding official Debian libffi 3.4.8-2 locally.

AI assistance: OpenAI Codex (model version unavailable).

@youknowdot
Isolate GC ownership test from child automatic collection
14d7aa9 
Disable automatic GC before creating the child cycle so the test measures only whether the parent forced collection can reach child-owned objects. Keep both assertions and the parent collection retry unchanged.

Assisted-by: OpenAI Codex:model version unavailable
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •
edited
Loading

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
  • Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5ec560f8-f856-4398-8c9e-d24ab400af2f
📥 Commits

Reviewing files that changed from the base of the PR and between e7e3dfd and 14d7aa9.

📒 Files selected for processing: 1
  • crates/vm/src/vm/interpreter.rs
+-----------------------------------------------------------------------------------------------------------------------------------------------------------+
| Don't be a slave to formal methods. Don't blindly adopt any technique without putting it into the context of your development practices and capabilities. |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ
✨ 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

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

@codspeed

codspeed Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing youknowdot:fix/gc-ownership-test-isolation (14d7aa9) with main (e7e3dfd)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@youknowdot youknowdot mentioned this pull request Oct 19, 2026
Draft
@youknowone
youknowone marked this pull request as ready for review October 19, 2026 11:12
@youknowone
youknowone merged commit 23da4ac into RustPython:main Oct 19, 2026
22 checks passed
@chatgpt-codex-connector

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

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-19T11:14:35.476261Z 14d7aa9 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.

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@youknowdot @youknowone