Skip to content

Commit bee112a

Browse files
Eclips4rhansenvstinner
authored
gh-124872: Replace enter/exit events with "switched" (#125532)
Users want to know when the current context switches to a different context object. Right now this happens when and only when a context is entered or exited, so the enter and exit events are synonymous with "switched". However, if the changes proposed for gh-99633 are implemented, the current context will also switch for reasons other than context enter or exit. Since users actually care about context switches and not enter or exit, replace the enter and exit events with a single switched event. The former exit event was emitted just before exiting the context. The new switched event is emitted after the context is exited to match the semantics users expect of an event with a past-tense name. If users need the ability to clean up before the switch takes effect, another event type can be added in the future. It is not added here because YAGNI. I skipped 0 in the enum as a matter of practice. Skipping 0 makes it easier to troubleshoot when code forgets to set zeroed memory, and it aligns with best practices for other tools (e.g., https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum). Co-authored-by: Richard Hansen <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
1 parent 37e533a commit bee112a

File tree

6 files changed

+118
-114
lines changed

6 files changed

+118
-114
lines changed

Doc/c-api/contextvars.rst

+4-10
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,10 @@ Context object management functions:
123123
124124
Enumeration of possible context object watcher events:
125125
126-
- ``Py_CONTEXT_EVENT_ENTER``: A context has been entered, causing the
127-
:term:`current context` to switch to it. The object passed to the watch
128-
callback is the now-current :class:`contextvars.Context` object. Each
129-
enter event will eventually have a corresponding exit event for the same
130-
context object after any subsequently entered contexts have themselves been
131-
exited.
132-
- ``Py_CONTEXT_EVENT_EXIT``: A context is about to be exited, which will
133-
cause the :term:`current context` to switch back to what it was before the
134-
context was entered. The object passed to the watch callback is the
135-
still-current :class:`contextvars.Context` object.
126+
- ``Py_CONTEXT_SWITCHED``: The :term:`current context` has switched to a
127+
different context. The object passed to the watch callback is the
128+
now-current :class:`contextvars.Context` object, or None if no context is
129+
current.
136130
137131
.. versionadded:: 3.14
138132

Include/cpython/context.h

+4-13
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,11 @@ PyAPI_FUNC(int) PyContext_Exit(PyObject *);
2929

3030
typedef enum {
3131
/*
32-
* A context has been entered, causing the "current context" to switch to
33-
* it. The object passed to the watch callback is the now-current
34-
* contextvars.Context object. Each enter event will eventually have a
35-
* corresponding exit event for the same context object after any
36-
* subsequently entered contexts have themselves been exited.
32+
* The current context has switched to a different context. The object
33+
* passed to the watch callback is the now-current contextvars.Context
34+
* object, or None if no context is current.
3735
*/
38-
Py_CONTEXT_EVENT_ENTER,
39-
/*
40-
* A context is about to be exited, which will cause the "current context"
41-
* to switch back to what it was before the context was entered. The
42-
* object passed to the watch callback is the still-current
43-
* contextvars.Context object.
44-
*/
45-
Py_CONTEXT_EVENT_EXIT,
36+
Py_CONTEXT_SWITCHED = 1,
4637
} PyContextEvent;
4738

4839
/*

Lib/test/test_capi/test_watchers.py

+46-41
Original file line numberDiff line numberDiff line change
@@ -577,68 +577,66 @@ class TestContextObjectWatchers(unittest.TestCase):
577577
def context_watcher(self, which_watcher):
578578
wid = _testcapi.add_context_watcher(which_watcher)
579579
try:
580-
yield wid
580+
switches = _testcapi.get_context_switches(which_watcher)
581+
except ValueError:
582+
switches = None
583+
try:
584+
yield switches
581585
finally:
582586
_testcapi.clear_context_watcher(wid)
583587

584-
def assert_event_counts(self, exp_enter_0, exp_exit_0,
585-
exp_enter_1, exp_exit_1):
586-
self.assertEqual(
587-
exp_enter_0, _testcapi.get_context_watcher_num_enter_events(0))
588-
self.assertEqual(
589-
exp_exit_0, _testcapi.get_context_watcher_num_exit_events(0))
590-
self.assertEqual(
591-
exp_enter_1, _testcapi.get_context_watcher_num_enter_events(1))
592-
self.assertEqual(
593-
exp_exit_1, _testcapi.get_context_watcher_num_exit_events(1))
588+
def assert_event_counts(self, want_0, want_1):
589+
self.assertEqual(len(_testcapi.get_context_switches(0)), want_0)
590+
self.assertEqual(len(_testcapi.get_context_switches(1)), want_1)
594591

595592
def test_context_object_events_dispatched(self):
596593
# verify that all counts are zero before any watchers are registered
597-
self.assert_event_counts(0, 0, 0, 0)
594+
self.assert_event_counts(0, 0)
598595

599596
# verify that all counts remain zero when a context object is
600597
# entered and exited with no watchers registered
601598
ctx = contextvars.copy_context()
602-
ctx.run(self.assert_event_counts, 0, 0, 0, 0)
603-
self.assert_event_counts(0, 0, 0, 0)
599+
ctx.run(self.assert_event_counts, 0, 0)
600+
self.assert_event_counts(0, 0)
604601

605602
# verify counts are as expected when first watcher is registered
606603
with self.context_watcher(0):
607-
self.assert_event_counts(0, 0, 0, 0)
608-
ctx.run(self.assert_event_counts, 1, 0, 0, 0)
609-
self.assert_event_counts(1, 1, 0, 0)
604+
self.assert_event_counts(0, 0)
605+
ctx.run(self.assert_event_counts, 1, 0)
606+
self.assert_event_counts(2, 0)
610607

611608
# again with second watcher registered
612609
with self.context_watcher(1):
613-
self.assert_event_counts(1, 1, 0, 0)
614-
ctx.run(self.assert_event_counts, 2, 1, 1, 0)
615-
self.assert_event_counts(2, 2, 1, 1)
610+
self.assert_event_counts(2, 0)
611+
ctx.run(self.assert_event_counts, 3, 1)
612+
self.assert_event_counts(4, 2)
616613

617614
# verify counts are reset and don't change after both watchers are cleared
618-
ctx.run(self.assert_event_counts, 0, 0, 0, 0)
619-
self.assert_event_counts(0, 0, 0, 0)
615+
ctx.run(self.assert_event_counts, 0, 0)
616+
self.assert_event_counts(0, 0)
620617

621-
def test_enter_error(self):
622-
with self.context_watcher(2):
623-
with catch_unraisable_exception() as cm:
624-
ctx = contextvars.copy_context()
625-
ctx.run(int, 0)
626-
self.assertEqual(
627-
cm.unraisable.err_msg,
628-
"Exception ignored in "
629-
f"Py_CONTEXT_EVENT_EXIT watcher callback for {ctx!r}"
630-
)
631-
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
618+
def test_callback_error(self):
619+
ctx_outer = contextvars.copy_context()
620+
ctx_inner = contextvars.copy_context()
621+
unraisables = []
632622

633-
def test_exit_error(self):
634-
ctx = contextvars.copy_context()
635-
def _in_context(stack):
636-
stack.enter_context(self.context_watcher(2))
623+
def _in_outer():
624+
with self.context_watcher(2):
625+
with catch_unraisable_exception() as cm:
626+
ctx_inner.run(lambda: unraisables.append(cm.unraisable))
627+
unraisables.append(cm.unraisable)
637628

638-
with catch_unraisable_exception() as cm:
639-
with ExitStack() as stack:
640-
ctx.run(_in_context, stack)
641-
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
629+
try:
630+
ctx_outer.run(_in_outer)
631+
self.assertEqual([x.err_msg for x in unraisables],
632+
["Exception ignored in Py_CONTEXT_SWITCHED "
633+
f"watcher callback for {ctx!r}"
634+
for ctx in [ctx_inner, ctx_outer]])
635+
self.assertEqual([str(x.exc_value) for x in unraisables],
636+
["boom!", "boom!"])
637+
finally:
638+
# Break reference cycle
639+
unraisables = None
642640

643641
def test_clear_out_of_range_watcher_id(self):
644642
with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"):
@@ -654,5 +652,12 @@ def test_allocate_too_many_watchers(self):
654652
with self.assertRaisesRegex(RuntimeError, r"no more context watcher IDs available"):
655653
_testcapi.allocate_too_many_context_watchers()
656654

655+
def test_exit_base_context(self):
656+
ctx = contextvars.Context()
657+
_testcapi.clear_context_stack()
658+
with self.context_watcher(0) as switches:
659+
ctx.run(lambda: None)
660+
self.assertEqual(switches, [ctx, None])
661+
657662
if __name__ == "__main__":
658663
unittest.main()

Modules/_testcapi/watchers.c

+41-38
Original file line numberDiff line numberDiff line change
@@ -626,16 +626,12 @@ allocate_too_many_func_watchers(PyObject *self, PyObject *args)
626626
// Test contexct object watchers
627627
#define NUM_CONTEXT_WATCHERS 2
628628
static int context_watcher_ids[NUM_CONTEXT_WATCHERS] = {-1, -1};
629-
static int num_context_object_enter_events[NUM_CONTEXT_WATCHERS] = {0, 0};
630-
static int num_context_object_exit_events[NUM_CONTEXT_WATCHERS] = {0, 0};
629+
static PyObject *context_switches[NUM_CONTEXT_WATCHERS];
631630

632631
static int
633632
handle_context_watcher_event(int which_watcher, PyContextEvent event, PyObject *ctx) {
634-
if (event == Py_CONTEXT_EVENT_ENTER) {
635-
num_context_object_enter_events[which_watcher]++;
636-
}
637-
else if (event == Py_CONTEXT_EVENT_EXIT) {
638-
num_context_object_exit_events[which_watcher]++;
633+
if (event == Py_CONTEXT_SWITCHED) {
634+
PyList_Append(context_switches[which_watcher], ctx);
639635
}
640636
else {
641637
return -1;
@@ -667,31 +663,28 @@ error_context_event_handler(PyContextEvent event, PyObject *ctx) {
667663
static PyObject *
668664
add_context_watcher(PyObject *self, PyObject *which_watcher)
669665
{
670-
int watcher_id;
666+
static const PyContext_WatchCallback callbacks[] = {
667+
&first_context_watcher_callback,
668+
&second_context_watcher_callback,
669+
&error_context_event_handler,
670+
};
671671
assert(PyLong_Check(which_watcher));
672672
long which_l = PyLong_AsLong(which_watcher);
673-
if (which_l == 0) {
674-
watcher_id = PyContext_AddWatcher(first_context_watcher_callback);
675-
context_watcher_ids[0] = watcher_id;
676-
num_context_object_enter_events[0] = 0;
677-
num_context_object_exit_events[0] = 0;
678-
}
679-
else if (which_l == 1) {
680-
watcher_id = PyContext_AddWatcher(second_context_watcher_callback);
681-
context_watcher_ids[1] = watcher_id;
682-
num_context_object_enter_events[1] = 0;
683-
num_context_object_exit_events[1] = 0;
684-
}
685-
else if (which_l == 2) {
686-
watcher_id = PyContext_AddWatcher(error_context_event_handler);
687-
}
688-
else {
673+
if (which_l < 0 || which_l >= (long)Py_ARRAY_LENGTH(callbacks)) {
689674
PyErr_Format(PyExc_ValueError, "invalid watcher %d", which_l);
690675
return NULL;
691676
}
677+
int watcher_id = PyContext_AddWatcher(callbacks[which_l]);
692678
if (watcher_id < 0) {
693679
return NULL;
694680
}
681+
if (which_l >= 0 && which_l < NUM_CONTEXT_WATCHERS) {
682+
context_watcher_ids[which_l] = watcher_id;
683+
Py_XSETREF(context_switches[which_l], PyList_New(0));
684+
if (context_switches[which_l] == NULL) {
685+
return NULL;
686+
}
687+
}
695688
return PyLong_FromLong(watcher_id);
696689
}
697690

@@ -708,30 +701,42 @@ clear_context_watcher(PyObject *self, PyObject *watcher_id)
708701
for (int i = 0; i < NUM_CONTEXT_WATCHERS; i++) {
709702
if (watcher_id_l == context_watcher_ids[i]) {
710703
context_watcher_ids[i] = -1;
711-
num_context_object_enter_events[i] = 0;
712-
num_context_object_exit_events[i] = 0;
704+
Py_CLEAR(context_switches[i]);
713705
}
714706
}
715707
}
716708
Py_RETURN_NONE;
717709
}
718710

719711
static PyObject *
720-
get_context_watcher_num_enter_events(PyObject *self, PyObject *watcher_id)
712+
clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
721713
{
722-
assert(PyLong_Check(watcher_id));
723-
long watcher_id_l = PyLong_AsLong(watcher_id);
724-
assert(watcher_id_l >= 0 && watcher_id_l < NUM_CONTEXT_WATCHERS);
725-
return PyLong_FromLong(num_context_object_enter_events[watcher_id_l]);
714+
PyThreadState *tstate = PyThreadState_Get();
715+
if (tstate->context == NULL) {
716+
Py_RETURN_NONE;
717+
}
718+
if (((PyContext *)tstate->context)->ctx_prev != NULL) {
719+
PyErr_SetString(PyExc_RuntimeError,
720+
"must first exit all non-base contexts");
721+
return NULL;
722+
}
723+
Py_CLEAR(tstate->context);
724+
Py_RETURN_NONE;
726725
}
727726

728727
static PyObject *
729-
get_context_watcher_num_exit_events(PyObject *self, PyObject *watcher_id)
728+
get_context_switches(PyObject *Py_UNUSED(self), PyObject *watcher_id)
730729
{
731730
assert(PyLong_Check(watcher_id));
732731
long watcher_id_l = PyLong_AsLong(watcher_id);
733-
assert(watcher_id_l >= 0 && watcher_id_l < NUM_CONTEXT_WATCHERS);
734-
return PyLong_FromLong(num_context_object_exit_events[watcher_id_l]);
732+
if (watcher_id_l < 0 || watcher_id_l >= NUM_CONTEXT_WATCHERS) {
733+
PyErr_Format(PyExc_ValueError, "invalid watcher %ld", watcher_id_l);
734+
return NULL;
735+
}
736+
if (context_switches[watcher_id_l] == NULL) {
737+
return PyList_New(0);
738+
}
739+
return Py_NewRef(context_switches[watcher_id_l]);
735740
}
736741

737742
static PyObject *
@@ -835,10 +840,8 @@ static PyMethodDef test_methods[] = {
835840
// Code object watchers.
836841
{"add_context_watcher", add_context_watcher, METH_O, NULL},
837842
{"clear_context_watcher", clear_context_watcher, METH_O, NULL},
838-
{"get_context_watcher_num_enter_events",
839-
get_context_watcher_num_enter_events, METH_O, NULL},
840-
{"get_context_watcher_num_exit_events",
841-
get_context_watcher_num_exit_events, METH_O, NULL},
843+
{"clear_context_stack", clear_context_stack, METH_NOARGS, NULL},
844+
{"get_context_switches", get_context_switches, METH_O, NULL},
842845
{"allocate_too_many_context_watchers",
843846
(PyCFunction) allocate_too_many_context_watchers, METH_NOARGS, NULL},
844847
{NULL},

Python/context.c

+21-10
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,8 @@ PyContext_CopyCurrent(void)
102102
static const char *
103103
context_event_name(PyContextEvent event) {
104104
switch (event) {
105-
case Py_CONTEXT_EVENT_ENTER:
106-
return "Py_CONTEXT_EVENT_ENTER";
107-
case Py_CONTEXT_EVENT_EXIT:
108-
return "Py_CONTEXT_EVENT_EXIT";
105+
case Py_CONTEXT_SWITCHED:
106+
return "Py_CONTEXT_SWITCHED";
109107
default:
110108
return "?";
111109
}
@@ -115,6 +113,13 @@ context_event_name(PyContextEvent event) {
115113
static void
116114
notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
117115
{
116+
if (ctx == NULL) {
117+
// This will happen after exiting the last context in the stack, which
118+
// can occur if context_get was never called before entering a context
119+
// (e.g., called `contextvars.Context().run()` on a fresh thread, as
120+
// PyContext_Enter doesn't call context_get).
121+
ctx = Py_None;
122+
}
118123
assert(Py_REFCNT(ctx) > 0);
119124
PyInterpreterState *interp = ts->interp;
120125
assert(interp->_initialized);
@@ -175,6 +180,16 @@ PyContext_ClearWatcher(int watcher_id)
175180
}
176181

177182

183+
static inline void
184+
context_switched(PyThreadState *ts)
185+
{
186+
ts->context_ver++;
187+
// ts->context is used instead of context_get() because context_get() might
188+
// throw if ts->context is NULL.
189+
notify_context_watchers(ts, Py_CONTEXT_SWITCHED, ts->context);
190+
}
191+
192+
178193
static int
179194
_PyContext_Enter(PyThreadState *ts, PyObject *octx)
180195
{
@@ -191,9 +206,7 @@ _PyContext_Enter(PyThreadState *ts, PyObject *octx)
191206
ctx->ctx_entered = 1;
192207

193208
ts->context = Py_NewRef(ctx);
194-
ts->context_ver++;
195-
196-
notify_context_watchers(ts, Py_CONTEXT_EVENT_ENTER, octx);
209+
context_switched(ts);
197210
return 0;
198211
}
199212

@@ -227,13 +240,11 @@ _PyContext_Exit(PyThreadState *ts, PyObject *octx)
227240
return -1;
228241
}
229242

230-
notify_context_watchers(ts, Py_CONTEXT_EVENT_EXIT, octx);
231243
Py_SETREF(ts->context, (PyObject *)ctx->ctx_prev);
232-
ts->context_ver++;
233244

234245
ctx->ctx_prev = NULL;
235246
ctx->ctx_entered = 0;
236-
247+
context_switched(ts);
237248
return 0;
238249
}
239250

Tools/c-analyzer/cpython/ignored.tsv

+2-2
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,8 @@ Modules/_testcapi/watchers.c - pyfunc_watchers -
455455
Modules/_testcapi/watchers.c - func_watcher_ids -
456456
Modules/_testcapi/watchers.c - func_watcher_callbacks -
457457
Modules/_testcapi/watchers.c - context_watcher_ids -
458-
Modules/_testcapi/watchers.c - num_context_object_enter_events -
459-
Modules/_testcapi/watchers.c - num_context_object_exit_events -
458+
Modules/_testcapi/watchers.c - context_switches -
459+
Modules/_testcapi/watchers.c add_context_watcher callbacks -
460460
Modules/_testcapimodule.c - BasicStaticTypes -
461461
Modules/_testcapimodule.c - num_basic_static_types_used -
462462
Modules/_testcapimodule.c - ContainerNoGC_members -

0 commit comments

Comments
 (0)