Skip to content

Commit

Permalink
gh-119333, gh-99633: Replace enter/exit events with "switched"
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
rhansen committed Sep 30, 2024
1 parent ebac569 commit 0b18cf1
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 102 deletions.
13 changes: 3 additions & 10 deletions Doc/c-api/contextvars.rst
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,9 @@ 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.
.. 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.
*/
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
78 changes: 35 additions & 43 deletions Lib/test/test_capi/test_watchers.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,64 +581,56 @@ def context_watcher(self, which_watcher):
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, exp_switched_0, exp_switched_1):
self.assertEqual(exp_switched_0,
_testcapi.get_context_watcher_num_switched_events(0))
self.assertEqual(exp_switched_1,
_testcapi.get_context_watcher_num_switched_events(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_exception_save(self):
with self.context_watcher(2):
Expand Down
38 changes: 10 additions & 28 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 int num_context_object_switched_events[NUM_CONTEXT_WATCHERS] = {0, 0};

static void
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) {
num_context_object_switched_events[which_watcher]++;
}
else {
Py_UNREACHABLE();
Expand Down Expand Up @@ -670,14 +666,12 @@ add_context_watcher(PyObject *self, PyObject *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;
num_context_object_switched_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;
num_context_object_switched_events[1] = 0;
}
else if (which_l == 2) {
watcher_id = PyContext_AddWatcher(error_context_event_handler);
Expand Down Expand Up @@ -705,30 +699,20 @@ 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;
num_context_object_switched_events[i] = 0;
}
}
}
Py_RETURN_NONE;
}

static PyObject *
get_context_watcher_num_enter_events(PyObject *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_enter_events[watcher_id_l]);
}

static PyObject *
get_context_watcher_num_exit_events(PyObject *self, PyObject *watcher_id)
get_context_watcher_num_switched_events(PyObject *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]);
return PyLong_FromLong(num_context_object_switched_events[watcher_id_l]);
}

static PyObject *
Expand Down Expand Up @@ -832,10 +816,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},
{"get_context_watcher_num_switched_events",
get_context_watcher_num_switched_events, METH_O, NULL},
{"allocate_too_many_context_watchers",
(PyCFunction) allocate_too_many_context_watchers, METH_NOARGS, NULL},
{NULL},
Expand Down
14 changes: 6 additions & 8 deletions Python/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,16 @@ 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 "?";
}
Py_UNREACHABLE();
}

static void
notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyContext *ctx)
notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
{
assert(Py_REFCNT(ctx) > 0);
PyInterpreterState *interp = ts->interp;
Expand All @@ -126,7 +124,7 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyContext *ctx)
PyContext_WatchCallback cb = interp->context_watchers[i];
assert(cb != NULL);
PyObject *exc = _PyErr_GetRaisedException(ts);
cb(event, (PyObject *)ctx);
cb(event, ctx);
if (_PyErr_Occurred(ts) != NULL) {
PyErr_FormatUnraisable(
"Exception ignored in %s watcher callback for %R",
Expand Down Expand Up @@ -196,7 +194,7 @@ _PyContext_Enter(PyThreadState *ts, PyObject *octx)
ts->context = Py_NewRef(ctx);
ts->context_ver++;

notify_context_watchers(ts, Py_CONTEXT_EVENT_ENTER, ctx);
notify_context_watchers(ts, Py_CONTEXT_SWITCHED, ts->context);
return 0;
}

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

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

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

notify_context_watchers(ts, Py_CONTEXT_SWITCHED, ts->context);
return 0;
}

Expand Down

0 comments on commit 0b18cf1

Please sign in to comment.