Visitar URL original
Prevent ContextVar cache hits after Context address reuse by youknowdot · Pull Request #9009 · RustPython/RustPython · GitHub
Skip to content

Prevent ContextVar cache hits after Context address reuse - #9009

Merged
youknowone merged 3 commits into
RustPython:mainfrom
youknowdot:fix-context-cache-identity
Oct 9, 2026
Merged

youknowone merged 3 commits into
RustPython:mainfrom
youknowdot:fix-context-cache-identity

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Summary

Extract the ContextVar cache-identity fix from #8954 for the Python 3.14 main branch.

A ContextVar can retain a cached value after its Context is destroyed. If a new empty Context reuses that address at the same stack depth, the old address-based cache check can return the destroyed Context's value even though the new mapping is empty.

Give every Context, including copies, a non-reused cache identity. If the identity counter saturates, zero disables cache hits. Existing cache locking and replacement/drop ordering stay intact. Add a Rust interpreter regression for fresh and copied contexts, variable defaults, explicit defaults, and missing-value errors.

This changes one implementation file and adds one integration test. Canonical Python tests, dependencies and build profiles are unchanged.

Validation

Independently validated on main-based head 15cb422e0 with its own Python 3.14 library:

  • The exact new integration test fails on unpatched main and passes with the fix.
  • A batch-allocation probe confirms actual address reuse: unpatched main returns stale values at reused addresses; patched dev and release runs return zero stale values after reuse for both fresh Contexts and copies.
  • The initial shorter CLI probe observed no address reuse after the layout change and was retained as inconclusive. Its assertions were not weakened; the separate batch probe exercises the missing case.
  • Normal hooks and strict workspace/C-API Clippy passed. Workspace tests: 1,353 passed, 18 existing ignored. Separate C-API tests: 116 passed, 4 existing ignored.
  • Dev and production-feature release builds passed. Full unchanged test_context passes in both: 56 tests, one existing skip. Both existing contextvars/threading-contextvars snippets pass, and the release context suite also passes through cargo run.

Local execution was Linux only. Asyncio cohorts requiring the sandbox-denied AF_UNIX socketpair were not run; hosted tests remain enabled and cross-platform CI is pending. Counter exhaustion was source-reviewed without mutating the global counter. The available CPython comparison was 3.15.0rc3, not a CPython 3.14 reference run.

AI assistance

Codex assisted with implementation, extraction, source review, regression tests, and automated validation. The original implementation authorship and Assisted-by trailer are preserved; the regression commit also includes its disclosure.

Give each Context, including copies, a non-reused cache identity instead
of using its heap address. Disable cache hits if the identity counter
saturates. This prevents an empty Context from reading a destroyed
Context's cached value while preserving existing locking and drop order.

Assisted-by: Codex:model-version-unavailable
Exercise fresh and copied contexts with variable defaults, explicit defaults and missing values. The repeated lifecycle check complements canonical context tests without relying on a fixed allocator address-reuse count.

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

coderabbitai Bot commented Oct 8, 2026 •

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: 5ce5a164-7121-4e0b-ac92-a1106531a90b

📥 Commits

Reviewing files that changed from the base of the PR and between 54e47cd and 85361fa.


📒 Files selected for processing (2)
  • crates/stdlib/src/contextvars.rs
  • extra_tests/snippets/stdlib_contextvars.py

 _________________________________________
< OODA Loop: Observe, Orient, Debug, Act. >
 -----------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ 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.

Comment thread tests/context_cache.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not add python test code here. put it under extra_tests/snippets

Preserve the fresh and copied Context lifetime checks, defaults and missing-value assertions while removing the redundant Rust interpreter wrapper.

Assisted-by: Codex:model-version-unavailable
@youknowone
youknowone marked this pull request as ready for review October 9, 2026 09:27
@youknowone
youknowone merged commit cd623a9 into RustPython:main Oct 9, 2026
20 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

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-09T09:31:43.648970Z 85361fa 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85361fa4e6

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

] == []


def context_cache_does_not_outlive_its_context():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the prohibited test-body addition

This block adds a new test function, assertions, and test data to extra_tests/snippets/stdlib_contextvars.py, but the repository's critical test-code policy limits test-file edits to adding or removing expected-failure decorators and their associated TODOs. Remove this new test-body logic before landing the change.

AGENTS.md reference: AGENTS.md:L273-L279

Useful? React with 👍 / 👎.

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.

2 participants