Skip to content

Prevent ContextVar cache hits after Context address reuse #9009

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.

Sign up for GitHub

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
youknowone merged 3 commits into from
Oct 9, 2026
Merged

Prevent ContextVar cache hits after Context address reuse #9009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
568ffd1
Prevent ContextVar cache hits after Context address reuse
youknowdot Oct 7, 2026
15cb422
Cover ContextVar reads after context destruction
youknowdot Oct 8, 2026
85361fa
Move ContextVar cache regression into the contextvars snippet
youknowdot Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions crates/stdlib/src/contextvars.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,19 @@ mod _contextvars {
}
}

// Unlike an object address, a cache identity must not be reused after a
// context is destroyed. Zero disables cache hits if the counter is exhausted.
static CONTEXT_CACHE_ID: AtomicUsize = AtomicUsize::new(0);

fn next_context_cache_id() -> usize {
CONTEXT_CACHE_ID
.try_update(Ordering::Relaxed, Ordering::Relaxed, |id| id.checked_add(1))
.map_or(0, |id| id + 1)
}

#[derive(Debug)]
struct ContextInner {
cache_id: usize,
idx: AtomicUsize,
vars: PyRef<HamtObject>,
// PyObject *ctx_weakreflist;
Expand All @@ -72,6 +83,7 @@ mod _contextvars {
fn empty(vm: &VirtualMachine) -> Self {
Self {
inner: ContextInner {
cache_id: next_context_cache_id(),
idx: AtomicUsize::new(usize::MAX),
vars: HamtObject::default().into_ref(&vm.ctx),
entered: AtomicBool::new(false),
Expand Down Expand Up @@ -192,6 +204,7 @@ mod _contextvars {
};
Self {
inner: ContextInner {
cache_id: next_context_cache_id(),
idx: AtomicUsize::new(usize::MAX),
vars: vars_copy.into_ref(&vm.ctx),
entered: AtomicBool::new(false),
Expand Down Expand Up @@ -356,7 +369,7 @@ mod _contextvars {
#[pytraverse(skip)]
cached: PyMutex<Option<ContextVarCache>>,
#[pytraverse(skip)]
cached_id: AtomicUsize, // cached_tsid in CPython
cached_id: AtomicUsize, // non-reused Context cache identity
#[pytraverse(skip)]
hash: AtomicI64,
}
Expand Down Expand Up @@ -413,7 +426,7 @@ mod _contextvars {
idx: ctx.inner.idx.load(Ordering::Relaxed),
};
let mut cached = zelf.cached.lock();
zelf.cached_id.store(ctx.get_id(), Ordering::SeqCst);
zelf.cached_id.store(ctx.inner.cache_id, Ordering::SeqCst);
let replaced = cached.replace(cache);
drop(cached);
drop(replaced);
Expand Down Expand Up @@ -444,15 +457,16 @@ mod _contextvars {
let ctx = ctx_obj.downcast_ref::<PyContext>().unwrap();
let mut cached = zelf.cached.lock();
if let Some(cached) = &*cached
&& zelf.cached_id.load(Ordering::SeqCst) == ctx.get_id()
&& ctx.inner.cache_id != 0
&& zelf.cached_id.load(Ordering::SeqCst) == ctx.inner.cache_id
&& cached.idx + 1 == ctxs.len()
{
return (Some(cached.object.clone()), None);
}
let Some(obj) = ctx.borrow_vars().get(zelf).map(|obj| obj.to_owned()) else {
return (None, None);
};
zelf.cached_id.store(ctx.get_id(), Ordering::SeqCst);
zelf.cached_id.store(ctx.inner.cache_id, Ordering::SeqCst);

let replaced = cached.replace(ContextVarCache {
object: obj.clone(),
Expand Down
32 changes: 32 additions & 0 deletions extra_tests/snippets/stdlib_contextvars.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,35 @@ def __eq__(self, other):
for var, value in copy_context().items()
if var.name == "child_marker"
] == []


def context_cache_does_not_outlive_its_context():

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

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 👍 / 👎.

import contextvars

for factory_name in ("new", "copy"):
for _ in range(100):
var = contextvars.ContextVar("var", default="default")
empty = contextvars.Context()
ctx = contextvars.Context() if factory_name == "new" else empty.copy()
ctx.run(var.set, "cached")
del ctx
ctx = contextvars.Context() if factory_name == "new" else empty.copy()
assert list(ctx.items()) == []
assert ctx.run(var.get) == "default"
assert ctx.run(var.get, "explicit default") == "explicit default"

missing = contextvars.ContextVar("missing")
ctx.run(missing.set, "cached")
del ctx
ctx = contextvars.Context() if factory_name == "new" else empty.copy()
try:
ctx.run(missing.get)
except LookupError:
pass
else:
raise AssertionError(
"an empty context returned a destroyed context value"
)


context_cache_does_not_outlive_its_context()
Loading