Skip to content
Draft
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion Python/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,16 @@ contextvar_set(PyContextVar *var, PyObject *val)
return -1;
}

// gh-148891: _PyHamt_Assoc can allocate HAMT nodes, which may trigger
// GC. A weakref callback or finalizer could then re-enter
// contextvar_set, replacing ctx->ctx_vars via Py_SETREF and freeing
// the old HAMT while _PyHamt_Assoc is still traversing it.
// Prevent this by holding a strong reference to the current vars.
PyHamtObject *old_vars = ctx->ctx_vars;
Py_INCREF(old_vars);
PyHamtObject *new_vars = _PyHamt_Assoc(
ctx->ctx_vars, (PyObject *)var, val);
old_vars, (PyObject *)var, val);
Py_DECREF(old_vars);
Comment on lines +794 to +803
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.

This looks true. We have a lot of increfs/decrefs like that because the call between them can do scary things.

if (new_vars == NULL) {
return -1;
}
Expand All @@ -819,8 +827,12 @@ contextvar_del(PyContextVar *var)
return -1;
}

// gh-148891: same borrowed-reference hazard as contextvar_set:
// _PyHamt_Without allocates nodes, which may trigger GC re-entrancy.
PyHamtObject *vars = ctx->ctx_vars;
Py_INCREF(vars);
PyHamtObject *new_vars = _PyHamt_Without(vars, (PyObject *)var);
Py_DECREF(vars);
if (new_vars == NULL) {
return -1;
}
Expand Down
Loading