Skip to content

Commit 9ee248c

Browse files
authored
gh-155519: fix data-race for Context.ctx_vars (#155522)
1 parent 50fcb91 commit 9ee248c

3 files changed

Lines changed: 151 additions & 21 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import contextvars
2+
import unittest
3+
from threading import Event, Thread
4+
5+
from test.support import threading_helper
6+
7+
8+
@threading_helper.requires_working_threading()
9+
class TestContext(unittest.TestCase):
10+
def test_racing_read_write(self):
11+
# gh-154535: reading a Context object from one thread while another
12+
# thread sets variables in it used to crash. The readers looked at
13+
# Context.ctx_vars without owning a reference to it, so the writer
14+
# could deallocate the mapping while a reader was walking it.
15+
ctx = contextvars.Context()
16+
cvars = [contextvars.ContextVar(f"cvar{i}") for i in range(64)]
17+
done = Event()
18+
errors = []
19+
20+
def writer():
21+
def body():
22+
i = 0
23+
while not done.is_set():
24+
cvars[i % len(cvars)].set(i)
25+
i += 1
26+
try:
27+
ctx.run(body)
28+
except BaseException as e:
29+
errors.append(e)
30+
31+
def reader():
32+
try:
33+
for _ in range(200):
34+
ctx.copy()
35+
len(ctx)
36+
list(ctx)
37+
list(ctx.items())
38+
list(ctx.keys())
39+
list(ctx.values())
40+
cvars[0] in ctx
41+
ctx.get(cvars[0])
42+
ctx == ctx
43+
except BaseException as e:
44+
errors.append(e)
45+
finally:
46+
done.set()
47+
48+
threads = [Thread(target=writer)]
49+
threads += [Thread(target=reader) for _ in range(4)]
50+
with threading_helper.start_threads(threads, done.set):
51+
pass
52+
53+
self.assertEqual(errors, [], msg=f"unexpected errors: {errors}")
54+
55+
56+
if __name__ == "__main__":
57+
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Avoid a data-race in free-threaded builds when reading and writing context
2+
variables from different threads.

Python/context.c

Lines changed: 92 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#include "Python.h"
22
#include "pycore_call.h" // _PyObject_VectorcallTstate()
33
#include "pycore_context.h"
4+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
45
#include "pycore_freelist.h" // _Py_FREELIST_FREE(), _Py_FREELIST_POP()
56
#include "pycore_gc.h" // _PyObject_GC_MAY_BE_TRACKED()
67
#include "pycore_hamt.h"
78
#include "pycore_initconfig.h" // _PyStatus_OK()
89
#include "pycore_object.h"
10+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT_RELAXED()
911
#include "pycore_pyerrors.h"
1012
#include "pycore_pystate.h" // _PyThreadState_GET()
1113

@@ -64,6 +66,41 @@ contextvar_set(PyContextVar *var, PyObject *val);
6466
static int
6567
contextvar_del(PyContextVar *var);
6668

69+
static inline PyHamtObject *
70+
context_get_vars(PyContext *ctx)
71+
{
72+
PyHamtObject *vars;
73+
Py_BEGIN_CRITICAL_SECTION(ctx);
74+
vars = ctx->ctx_vars;
75+
assert(vars != NULL);
76+
Py_INCREF(vars);
77+
Py_END_CRITICAL_SECTION();
78+
return vars;
79+
}
80+
81+
static inline PyHamtObject *
82+
context_get_current_vars(PyContext *ctx)
83+
{
84+
// ctx_vars written only by the owning thread, and read by other threads
85+
// only under the context's lock, a plain (non-atomic) load is okay
86+
PyHamtObject *vars = ctx->ctx_vars;
87+
assert(vars != NULL);
88+
return vars;
89+
}
90+
91+
// Note: steals a reference to new_vars and must only be called by the thread
92+
// that has `ctx` as its current context.
93+
static inline void
94+
context_set_vars(PyContext *ctx, PyHamtObject *new_vars)
95+
{
96+
PyHamtObject *old_vars;
97+
Py_BEGIN_CRITICAL_SECTION(ctx);
98+
old_vars = ctx->ctx_vars;
99+
ctx->ctx_vars = new_vars;
100+
Py_END_CRITICAL_SECTION();
101+
Py_XDECREF(old_vars);
102+
}
103+
67104

68105
PyObject *
69106
_PyContext_NewHamtForTests(void)
@@ -84,7 +121,10 @@ PyContext_Copy(PyObject * octx)
84121
{
85122
ENSURE_Context(octx, NULL)
86123
PyContext *ctx = (PyContext *)octx;
87-
return (PyObject *)context_new_from_vars(ctx->ctx_vars);
124+
PyHamtObject *vars = context_get_vars(ctx);
125+
PyObject *res = (PyObject *)context_new_from_vars(vars);
126+
Py_DECREF(vars);
127+
return res;
88128
}
89129

90130

@@ -96,7 +136,7 @@ PyContext_CopyCurrent(void)
96136
return NULL;
97137
}
98138

99-
return (PyObject *)context_new_from_vars(ctx->ctx_vars);
139+
return (PyObject *)context_new_from_vars(context_get_current_vars(ctx));
100140
}
101141

102142
static const char *
@@ -298,7 +338,7 @@ PyContextVar_Get(PyObject *ovar, PyObject *def, PyObject **val)
298338
#endif
299339

300340
assert(PyContext_CheckExact(ts->context));
301-
PyHamtObject *vars = ((PyContext *)ts->context)->ctx_vars;
341+
PyHamtObject *vars = context_get_current_vars((PyContext *)ts->context);
302342

303343
PyObject *found = NULL;
304344
int res = _PyHamt_Find(vars, (PyObject*)var, &found);
@@ -354,7 +394,8 @@ PyContextVar_Set(PyObject *ovar, PyObject *val)
354394
}
355395

356396
PyObject *old_val = NULL;
357-
int found = _PyHamt_Find(ctx->ctx_vars, (PyObject *)var, &old_val);
397+
int found = _PyHamt_Find(context_get_current_vars(ctx), (PyObject *)var,
398+
&old_val);
358399
if (found < 0) {
359400
return NULL;
360401
}
@@ -552,7 +593,10 @@ static PyObject *
552593
context_tp_iter(PyObject *op)
553594
{
554595
PyContext *self = _PyContext_CAST(op);
555-
return _PyHamt_NewIterKeys(self->ctx_vars);
596+
PyHamtObject *vars = context_get_vars(self);
597+
PyObject *res = _PyHamt_NewIterKeys(vars);
598+
Py_DECREF(vars);
599+
return res;
556600
}
557601

558602
static PyObject *
@@ -564,8 +608,11 @@ context_tp_richcompare(PyObject *v, PyObject *w, int op)
564608
Py_RETURN_NOTIMPLEMENTED;
565609
}
566610

567-
int res = _PyHamt_Eq(
568-
((PyContext *)v)->ctx_vars, ((PyContext *)w)->ctx_vars);
611+
PyHamtObject *v_vars = context_get_vars((PyContext *)v);
612+
PyHamtObject *w_vars = context_get_vars((PyContext *)w);
613+
int res = _PyHamt_Eq(v_vars, w_vars);
614+
Py_DECREF(v_vars);
615+
Py_DECREF(w_vars);
569616
if (res < 0) {
570617
return NULL;
571618
}
@@ -586,7 +633,10 @@ static Py_ssize_t
586633
context_tp_len(PyObject *op)
587634
{
588635
PyContext *self = _PyContext_CAST(op);
589-
return _PyHamt_Len(self->ctx_vars);
636+
PyHamtObject *vars = context_get_vars(self);
637+
Py_ssize_t res = _PyHamt_Len(vars);
638+
Py_DECREF(vars);
639+
return res;
590640
}
591641

592642
static PyObject *
@@ -597,15 +647,18 @@ context_tp_subscript(PyObject *op, PyObject *key)
597647
}
598648
PyObject *val = NULL;
599649
PyContext *self = _PyContext_CAST(op);
600-
int found = _PyHamt_Find(self->ctx_vars, key, &val);
650+
PyHamtObject *vars = context_get_vars(self);
651+
int found = _PyHamt_Find(vars, key, &val);
652+
Py_XINCREF(val);
653+
Py_DECREF(vars);
601654
if (found < 0) {
602655
return NULL;
603656
}
604657
if (found == 0) {
605658
PyErr_SetObject(PyExc_KeyError, key);
606659
return NULL;
607660
}
608-
return Py_NewRef(val);
661+
return val;
609662
}
610663

611664
static int
@@ -616,7 +669,10 @@ context_tp_contains(PyObject *op, PyObject *key)
616669
}
617670
PyObject *val = NULL;
618671
PyContext *self = _PyContext_CAST(op);
619-
return _PyHamt_Find(self->ctx_vars, key, &val);
672+
PyHamtObject *vars = context_get_vars(self);
673+
int res = _PyHamt_Find(vars, key, &val);
674+
Py_DECREF(vars);
675+
return res;
620676
}
621677

622678

@@ -643,14 +699,17 @@ _contextvars_Context_get_impl(PyContext *self, PyObject *key,
643699
}
644700

645701
PyObject *val = NULL;
646-
int found = _PyHamt_Find(self->ctx_vars, key, &val);
702+
PyHamtObject *vars = context_get_vars(self);
703+
int found = _PyHamt_Find(vars, key, &val);
704+
Py_XINCREF(val);
705+
Py_DECREF(vars);
647706
if (found < 0) {
648707
return NULL;
649708
}
650709
if (found == 0) {
651710
return Py_NewRef(default_value);
652711
}
653-
return Py_NewRef(val);
712+
return val;
654713
}
655714

656715

@@ -666,7 +725,10 @@ static PyObject *
666725
_contextvars_Context_items_impl(PyContext *self)
667726
/*[clinic end generated code: output=fa1655c8a08502af input=00db64ae379f9f42]*/
668727
{
669-
return _PyHamt_NewIterItems(self->ctx_vars);
728+
PyHamtObject *vars = context_get_vars(self);
729+
PyObject *res = _PyHamt_NewIterItems(vars);
730+
Py_DECREF(vars);
731+
return res;
670732
}
671733

672734

@@ -680,7 +742,10 @@ static PyObject *
680742
_contextvars_Context_keys_impl(PyContext *self)
681743
/*[clinic end generated code: output=177227c6b63ec0e2 input=114b53aebca3449c]*/
682744
{
683-
return _PyHamt_NewIterKeys(self->ctx_vars);
745+
PyHamtObject *vars = context_get_vars(self);
746+
PyObject *res = _PyHamt_NewIterKeys(vars);
747+
Py_DECREF(vars);
748+
return res;
684749
}
685750

686751

@@ -694,7 +759,10 @@ static PyObject *
694759
_contextvars_Context_values_impl(PyContext *self)
695760
/*[clinic end generated code: output=d286dabfc8db6dde input=ce8075d04a6ea526]*/
696761
{
697-
return _PyHamt_NewIterValues(self->ctx_vars);
762+
PyHamtObject *vars = context_get_vars(self);
763+
PyObject *res = _PyHamt_NewIterValues(vars);
764+
Py_DECREF(vars);
765+
return res;
698766
}
699767

700768

@@ -708,7 +776,10 @@ static PyObject *
708776
_contextvars_Context_copy_impl(PyContext *self)
709777
/*[clinic end generated code: output=30ba8896c4707a15 input=ebafdbdd9c72d592]*/
710778
{
711-
return (PyObject *)context_new_from_vars(self->ctx_vars);
779+
PyHamtObject *vars = context_get_vars(self);
780+
PyObject *res = (PyObject *)context_new_from_vars(vars);
781+
Py_DECREF(vars);
782+
return res;
712783
}
713784

714785

@@ -796,12 +867,12 @@ contextvar_set(PyContextVar *var, PyObject *val)
796867
}
797868

798869
PyHamtObject *new_vars = _PyHamt_Assoc(
799-
ctx->ctx_vars, (PyObject *)var, val);
870+
context_get_current_vars(ctx), (PyObject *)var, val);
800871
if (new_vars == NULL) {
801872
return -1;
802873
}
803874

804-
Py_SETREF(ctx->ctx_vars, new_vars);
875+
context_set_vars(ctx, new_vars);
805876

806877
#ifndef Py_GIL_DISABLED
807878
var->var_cached = val; /* borrow */
@@ -823,7 +894,7 @@ contextvar_del(PyContextVar *var)
823894
return -1;
824895
}
825896

826-
PyHamtObject *vars = ctx->ctx_vars;
897+
PyHamtObject *vars = context_get_current_vars(ctx);
827898
PyHamtObject *new_vars = _PyHamt_Without(vars, (PyObject *)var);
828899
if (new_vars == NULL) {
829900
return -1;
@@ -835,7 +906,7 @@ contextvar_del(PyContextVar *var)
835906
return -1;
836907
}
837908

838-
Py_SETREF(ctx->ctx_vars, new_vars);
909+
context_set_vars(ctx, new_vars);
839910
return 0;
840911
}
841912

0 commit comments

Comments
 (0)