]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-124872: Replace enter/exit events with "switched" (#125532)
authorKirill Podoprigora <kirill.bast9@mail.ru>
Wed, 16 Oct 2024 11:53:21 +0000 (14:53 +0300)
committerGitHub <noreply@github.com>
Wed, 16 Oct 2024 11:53:21 +0000 (13:53 +0200)
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 <rhansen@rhansen.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Doc/c-api/contextvars.rst
Include/cpython/context.h
Lib/test/test_capi/test_watchers.py
Modules/_testcapi/watchers.c
Python/context.c
Tools/c-analyzer/cpython/ignored.tsv

index 8eba54a80dc80d47614a9bfa65430963374b3e90..b7c6550ff34aac1e4dd6ef5d26f0e77b085dee20 100644 (file)
@@ -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
 
index 3c9be7873b939900119d1be5310fd9c64456f4d6..3a7a4b459c09ad04d0dcd469b3cc556c36bf390f 100644 (file)
@@ -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;
 
 /*
index f21d2627c6094b6be9233f08bc9c4ce2def4ded0..4bb764bf9d0963e0f8a6d51d17570c3ee8c675a4 100644 (file)
@@ -577,68 +577,66 @@ 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)
+        ctx.run(self.assert_event_counts, 0, 0)
+        self.assert_event_counts(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_callback_error(self):
+        ctx_outer = contextvars.copy_context()
+        ctx_inner = contextvars.copy_context()
+        unraisables = []
 
-    def test_exit_error(self):
-        ctx = contextvars.copy_context()
-        def _in_context(stack):
-            stack.enter_context(self.context_watcher(2))
+        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)
 
-        with catch_unraisable_exception() as cm:
-            with ExitStack() as stack:
-                ctx.run(_in_context, stack)
-            self.assertEqual(str(cm.unraisable.exc_value), "boom!")
+        try:
+            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!"])
+        finally:
+            # Break reference cycle
+            unraisables = None
 
     def test_clear_out_of_range_watcher_id(self):
         with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"):
@@ -654,5 +652,12 @@ class TestContextObjectWatchers(unittest.TestCase):
         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()
index b4233d07134aea846b8878787c0c80d2cbddba73..321d3aeffb6ad18f364aaeb9dc6336508c6b1084 100644 (file)
@@ -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;
@@ -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);
 }
 
@@ -708,8 +701,7 @@ 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]);
             }
         }
     }
@@ -717,21 +709,34 @@ clear_context_watcher(PyObject *self, PyObject *watcher_id)
 }
 
 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 *
@@ -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},
index 8bc487a33c890bfd305635e86c36148d736d3399..95aa82206270f9829ff175e957a310a32ca22752 100644 (file)
@@ -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 "?";
     }
@@ -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);
@@ -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)
 {
@@ -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;
 }
 
@@ -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;
 }
 
index e6c599a2ac4a464a2011bd363c71f8133c82c37b..2605825d3d007804a20004490a0433a9dfb0bc59 100644 (file)
@@ -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   -