Skip to content

gh-124872: Replace enter/exit events with "switched" #124776

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.

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
merged 1 commit into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
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: 4 additions & 10 deletions Doc/c-api/contextvars.rst
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,10 @@ Context object management functions:

Enumeration of possible context object watcher events:

- ``Py_CONTEXT_EVENT_ENTER``: A context has been entered, causing the
:term:`current context` to switch to it. The object passed to the watch
callback is the now-current :class:`contextvars.Context` object. Each
enter event will eventually have a corresponding exit event for the same
context object after any subsequently entered contexts have themselves been
exited.
- ``Py_CONTEXT_EVENT_EXIT``: A context is about to be exited, which will
cause the :term:`current context` to switch back to what it was before the
context was entered. The object passed to the watch callback is the
still-current :class:`contextvars.Context` object.
- ``Py_CONTEXT_SWITCHED``: The :term:`current context` has switched to a
different context. The object passed to the watch callback is the
now-current :class:`contextvars.Context` object, or None if no context is
current.

.. versionadded:: 3.14

Expand Down
17 changes: 4 additions & 13 deletions Include/cpython/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,11 @@ PyAPI_FUNC(int) PyContext_Exit(PyObject *);

typedef enum {
/*
* A context has been entered, causing the "current context" to switch to
* it. The object passed to the watch callback is the now-current
* contextvars.Context object. Each enter event will eventually have a
* corresponding exit event for the same context object after any
* subsequently entered contexts have themselves been exited.
* The current context has switched to a different context. The object
* passed to the watch callback is the now-current contextvars.Context
* object, or None if no context is current.
*/
Py_CONTEXT_EVENT_ENTER,
/*
* A context is about to be exited, which will cause the "current context"
* to switch back to what it was before the context was entered. The
* object passed to the watch callback is the still-current
* contextvars.Context object.
*/
Py_CONTEXT_EVENT_EXIT,
Py_CONTEXT_SWITCHED = 1,
} PyContextEvent;

/*
Expand Down
89 changes: 45 additions & 44 deletions Lib/test/test_capi/test_watchers.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,68 +577,62 @@ class TestContextObjectWatchers(unittest.TestCase):
def context_watcher(self, which_watcher):
wid = _testcapi.add_context_watcher(which_watcher)
try:
yield wid
switches = _testcapi.get_context_switches(which_watcher)
except ValueError:
switches = None
try:
yield switches
finally:
_testcapi.clear_context_watcher(wid)

def assert_event_counts(self, exp_enter_0, exp_exit_0,
exp_enter_1, exp_exit_1):
self.assertEqual(
exp_enter_0, _testcapi.get_context_watcher_num_enter_events(0))
self.assertEqual(
exp_exit_0, _testcapi.get_context_watcher_num_exit_events(0))
self.assertEqual(
exp_enter_1, _testcapi.get_context_watcher_num_enter_events(1))
self.assertEqual(
exp_exit_1, _testcapi.get_context_watcher_num_exit_events(1))
def assert_event_counts(self, want_0, want_1):
self.assertEqual(len(_testcapi.get_context_switches(0)), want_0)
self.assertEqual(len(_testcapi.get_context_switches(1)), want_1)

def test_context_object_events_dispatched(self):
# verify that all counts are zero before any watchers are registered
self.assert_event_counts(0, 0, 0, 0)
self.assert_event_counts(0, 0)

# verify that all counts remain zero when a context object is
# entered and exited with no watchers registered
ctx = contextvars.copy_context()
ctx.run(self.assert_event_counts, 0, 0, 0, 0)
self.assert_event_counts(0, 0, 0, 0)
ctx.run(self.assert_event_counts, 0, 0)
self.assert_event_counts(0, 0)

# verify counts are as expected when first watcher is registered
with self.context_watcher(0):
self.assert_event_counts(0, 0, 0, 0)
ctx.run(self.assert_event_counts, 1, 0, 0, 0)
self.assert_event_counts(1, 1, 0, 0)
self.assert_event_counts(0, 0)
ctx.run(self.assert_event_counts, 1, 0)
self.assert_event_counts(2, 0)

# again with second watcher registered
with self.context_watcher(1):
self.assert_event_counts(1, 1, 0, 0)
ctx.run(self.assert_event_counts, 2, 1, 1, 0)
self.assert_event_counts(2, 2, 1, 1)
self.assert_event_counts(2, 0)
ctx.run(self.assert_event_counts, 3, 1)
self.assert_event_counts(4, 2)

# verify counts are reset and don't change after both watchers are cleared
ctx.run(self.assert_event_counts, 0, 0, 0, 0)
self.assert_event_counts(0, 0, 0, 0)

def test_enter_error(self):
with self.context_watcher(2):
with catch_unraisable_exception() as cm:
ctx = contextvars.copy_context()
ctx.run(int, 0)
self.assertEqual(
cm.unraisable.err_msg,
"Exception ignored in "
f"Py_CONTEXT_EVENT_EXIT watcher callback for {ctx!r}"
)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")

def test_exit_error(self):
ctx = contextvars.copy_context()
def _in_context(stack):
stack.enter_context(self.context_watcher(2))

with catch_unraisable_exception() as cm:
with ExitStack() as stack:
ctx.run(_in_context, stack)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
ctx.run(self.assert_event_counts, 0, 0)
self.assert_event_counts(0, 0)

def test_callback_error(self):
ctx_outer = contextvars.copy_context()
ctx_inner = contextvars.copy_context()
unraisables = []

def _in_outer():
with self.context_watcher(2):
with catch_unraisable_exception() as cm:
ctx_inner.run(lambda: unraisables.append(cm.unraisable))
unraisables.append(cm.unraisable)

ctx_outer.run(_in_outer)
self.assertEqual([x.err_msg for x in unraisables],
["Exception ignored in Py_CONTEXT_SWITCHED "
f"watcher callback for {ctx!r}"
for ctx in [ctx_inner, ctx_outer]])
self.assertEqual([str(x.exc_value) for x in unraisables],
["boom!", "boom!"])

def test_clear_out_of_range_watcher_id(self):
with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"):
Expand All @@ -654,5 +648,12 @@ def test_allocate_too_many_watchers(self):
with self.assertRaisesRegex(RuntimeError, r"no more context watcher IDs available"):
_testcapi.allocate_too_many_context_watchers()

def test_exit_base_context(self):
ctx = contextvars.Context()
_testcapi.clear_context_stack()
with self.context_watcher(0) as switches:
ctx.run(lambda: None)
self.assertEqual(switches, [ctx, None])

if __name__ == "__main__":
unittest.main()
79 changes: 41 additions & 38 deletions Modules/_testcapi/watchers.c
Original file line number Diff line number Diff line change
Expand Up @@ -626,16 +626,12 @@ allocate_too_many_func_watchers(PyObject *self, PyObject *args)
// Test contexct object watchers
#define NUM_CONTEXT_WATCHERS 2
static int context_watcher_ids[NUM_CONTEXT_WATCHERS] = {-1, -1};
static int num_context_object_enter_events[NUM_CONTEXT_WATCHERS] = {0, 0};
static int num_context_object_exit_events[NUM_CONTEXT_WATCHERS] = {0, 0};
static PyObject *context_switches[NUM_CONTEXT_WATCHERS];

static int
handle_context_watcher_event(int which_watcher, PyContextEvent event, PyObject *ctx) {
if (event == Py_CONTEXT_EVENT_ENTER) {
num_context_object_enter_events[which_watcher]++;
}
else if (event == Py_CONTEXT_EVENT_EXIT) {
num_context_object_exit_events[which_watcher]++;
if (event == Py_CONTEXT_SWITCHED) {
PyList_Append(context_switches[which_watcher], ctx);
}
else {
return -1;
Expand Down Expand Up @@ -667,31 +663,28 @@ error_context_event_handler(PyContextEvent event, PyObject *ctx) {
static PyObject *
add_context_watcher(PyObject *self, PyObject *which_watcher)
{
int watcher_id;
static const PyContext_WatchCallback callbacks[] = {
&first_context_watcher_callback,
&second_context_watcher_callback,
&error_context_event_handler,
};
assert(PyLong_Check(which_watcher));
long which_l = PyLong_AsLong(which_watcher);
if (which_l == 0) {
watcher_id = PyContext_AddWatcher(first_context_watcher_callback);
context_watcher_ids[0] = watcher_id;
num_context_object_enter_events[0] = 0;
num_context_object_exit_events[0] = 0;
}
else if (which_l == 1) {
watcher_id = PyContext_AddWatcher(second_context_watcher_callback);
context_watcher_ids[1] = watcher_id;
num_context_object_enter_events[1] = 0;
num_context_object_exit_events[1] = 0;
}
else if (which_l == 2) {
watcher_id = PyContext_AddWatcher(error_context_event_handler);
}
else {
if (which_l < 0 || which_l >= (long)Py_ARRAY_LENGTH(callbacks)) {
PyErr_Format(PyExc_ValueError, "invalid watcher %d", which_l);
return NULL;
}
int watcher_id = PyContext_AddWatcher(callbacks[which_l]);
if (watcher_id < 0) {
return NULL;
}
if (which_l >= 0 && which_l < NUM_CONTEXT_WATCHERS) {
context_watcher_ids[which_l] = watcher_id;
Py_XSETREF(context_switches[which_l], PyList_New(0));
if (context_switches[which_l] == NULL) {
return NULL;
}
}
return PyLong_FromLong(watcher_id);
}

Expand All @@ -708,30 +701,42 @@ clear_context_watcher(PyObject *self, PyObject *watcher_id)
for (int i = 0; i < NUM_CONTEXT_WATCHERS; i++) {
if (watcher_id_l == context_watcher_ids[i]) {
context_watcher_ids[i] = -1;
num_context_object_enter_events[i] = 0;
num_context_object_exit_events[i] = 0;
Py_CLEAR(context_switches[i]);
}
}
}
Py_RETURN_NONE;
}

static PyObject *
get_context_watcher_num_enter_events(PyObject *self, PyObject *watcher_id)
clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
{
assert(PyLong_Check(watcher_id));
long watcher_id_l = PyLong_AsLong(watcher_id);
assert(watcher_id_l >= 0 && watcher_id_l < NUM_CONTEXT_WATCHERS);
return PyLong_FromLong(num_context_object_enter_events[watcher_id_l]);
PyThreadState *tstate = PyThreadState_Get();
if (tstate->context == NULL) {
Py_RETURN_NONE;
}
if (((PyContext *)tstate->context)->ctx_prev != NULL) {
PyErr_SetString(PyExc_RuntimeError,
"must first exit all non-base contexts");
return NULL;
}
Py_CLEAR(tstate->context);
Py_RETURN_NONE;
}

static PyObject *
get_context_watcher_num_exit_events(PyObject *self, PyObject *watcher_id)
get_context_switches(PyObject *Py_UNUSED(self), PyObject *watcher_id)
{
assert(PyLong_Check(watcher_id));
long watcher_id_l = PyLong_AsLong(watcher_id);
assert(watcher_id_l >= 0 && watcher_id_l < NUM_CONTEXT_WATCHERS);
return PyLong_FromLong(num_context_object_exit_events[watcher_id_l]);
if (watcher_id_l < 0 || watcher_id_l >= NUM_CONTEXT_WATCHERS) {
PyErr_Format(PyExc_ValueError, "invalid watcher %ld", watcher_id_l);
return NULL;
}
if (context_switches[watcher_id_l] == NULL) {
return PyList_New(0);
}
return Py_NewRef(context_switches[watcher_id_l]);
}

static PyObject *
Expand Down Expand Up @@ -835,10 +840,8 @@ static PyMethodDef test_methods[] = {
// Code object watchers.
{"add_context_watcher", add_context_watcher, METH_O, NULL},
{"clear_context_watcher", clear_context_watcher, METH_O, NULL},
{"get_context_watcher_num_enter_events",
get_context_watcher_num_enter_events, METH_O, NULL},
{"get_context_watcher_num_exit_events",
get_context_watcher_num_exit_events, METH_O, NULL},
{"clear_context_stack", clear_context_stack, METH_NOARGS, NULL},
{"get_context_switches", get_context_switches, METH_O, NULL},
{"allocate_too_many_context_watchers",
(PyCFunction) allocate_too_many_context_watchers, METH_NOARGS, NULL},
{NULL},
Expand Down
31 changes: 21 additions & 10 deletions Python/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,8 @@ PyContext_CopyCurrent(void)
static const char *
context_event_name(PyContextEvent event) {
switch (event) {
case Py_CONTEXT_EVENT_ENTER:
return "Py_CONTEXT_EVENT_ENTER";
case Py_CONTEXT_EVENT_EXIT:
return "Py_CONTEXT_EVENT_EXIT";
case Py_CONTEXT_SWITCHED:
return "Py_CONTEXT_SWITCHED";
default:
return "?";
Copy link
Member

Choose a reason for hiding this comment

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

Actually, please you use Py_UNREACHABLE() here

Copy link
Contributor

Choose a reason for hiding this comment

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

The ? was @gvanrossum idea in my pr.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, alright, I'm not gonna overturn Guido's review :) I'll push this as is.

}
Expand All @@ -115,6 +113,13 @@ context_event_name(PyContextEvent event) {
static void
notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
{
if (ctx == NULL) {
// This will happen after exiting the last context in the stack, which
// can occur if context_get was never called before entering a context
// (e.g., called `contextvars.Context().run()` on a fresh thread, as
// PyContext_Enter doesn't call context_get).
ctx = Py_None;
}
assert(Py_REFCNT(ctx) > 0);
PyInterpreterState *interp = ts->interp;
assert(interp->_initialized);
Expand Down Expand Up @@ -175,6 +180,16 @@ PyContext_ClearWatcher(int watcher_id)
}


static inline void
context_switched(PyThreadState *ts)
{
ts->context_ver++;
// ts->context is used instead of context_get() because context_get() might
// throw if ts->context is NULL.
notify_context_watchers(ts, Py_CONTEXT_SWITCHED, ts->context);
}


static int
_PyContext_Enter(PyThreadState *ts, PyObject *octx)
{
Expand All @@ -191,9 +206,7 @@ _PyContext_Enter(PyThreadState *ts, PyObject *octx)
ctx->ctx_entered = 1;

ts->context = Py_NewRef(ctx);
ts->context_ver++;

notify_context_watchers(ts, Py_CONTEXT_EVENT_ENTER, octx);
context_switched(ts);
return 0;
}

Expand Down Expand Up @@ -227,13 +240,11 @@ _PyContext_Exit(PyThreadState *ts, PyObject *octx)
return -1;
}

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

ctx->ctx_prev = NULL;
ctx->ctx_entered = 0;

context_switched(ts);
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions Tools/c-analyzer/cpython/ignored.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ Modules/_testcapi/watchers.c - pyfunc_watchers -
Modules/_testcapi/watchers.c - func_watcher_ids -
Modules/_testcapi/watchers.c - func_watcher_callbacks -
Modules/_testcapi/watchers.c - context_watcher_ids -
Modules/_testcapi/watchers.c - num_context_object_enter_events -
Modules/_testcapi/watchers.c - num_context_object_exit_events -
Modules/_testcapi/watchers.c - context_switches -
Modules/_testcapi/watchers.c add_context_watcher callbacks -
Modules/_testcapimodule.c - BasicStaticTypes -
Modules/_testcapimodule.c - num_basic_static_types_used -
Modules/_testcapimodule.c - ContainerNoGC_members -
Expand Down
Loading