diff --git a/Doc/c-api/contextvars.rst b/Doc/c-api/contextvars.rst index b7c6550ff34aac..306b6699837b1c 100644 --- a/Doc/c-api/contextvars.rst +++ b/Doc/c-api/contextvars.rst @@ -101,21 +101,76 @@ Context object management functions: current context for the current thread. Returns ``0`` on success, and ``-1`` on error. -.. c:function:: int PyContext_AddWatcher(PyContext_WatchCallback callback) - - Register *callback* as a context object watcher for the current interpreter. - Return an ID which may be passed to :c:func:`PyContext_ClearWatcher`. - In case of error (e.g. no more watcher IDs available), - return ``-1`` and set an exception. +.. c:function:: int PyContext_AddWatcher(PyObject *callback) + + Registers the callable object *callback* as a context object watcher for the + current interpreter. When a context event occurs, *callback* is called with + two arguments: + + #. An event type ID from :c:type:`PyContextEvent`. + #. An object containing event-specific supplemental data; see + :c:type:`PyContextEvent` for details. + + Any exception raised by *callback* will be printed as an unraisable + exception as if by a call to :c:func:`PyErr_FormatUnraisable`, then + discarded. + + On success, this function returns a non-negative ID which may be passed to + :c:func:`PyContext_ClearWatcher` to unregister the callback and remove the + reference this function adds to *callback*. Sets an exception and returns + ``-1`` on error (e.g., no more watcher IDs available). + + Example using a C function as the callback:: + + static PyObject * + my_callback(PyObject *self, PyObject *const *args, Py_ssize_t nargs) + { + if (PyVectorcall_NARGS(nargs) != 2) { + PyErr_Format(PyExc_TypeError, "want 2 args, got %zd", nargs); + return NULL; + } + int event = PyLong_AsInt(args[0]); + if (event == -1 && PyErr_Occurred()) { + return NULL; + } + if (event != Py_CONTEXT_SWITCHED) { + Py_RETURN_NONE; + } + PyObject *ctx = args[1]; + + // Do something interesting with self and ctx here. + + Py_RETURN_NONE; + } + + PyMethodDef my_callback_md = { + .ml_name = "my_callback", + .ml_meth = (PyCFunction)(void *)&my_callback, + .ml_flags = METH_FASTCALL, + .ml_doc = NULL, + }; + + int + register_my_callback(PyObject *callback_state) + { + PyObject *cb = PyCFunction_New(&my_callback_md, callback_state); + if (cb == NULL) { + return -1; + } + int id = PyContext_AddWatcher(cb); + Py_CLEAR(cb); + return id; + } .. versionadded:: 3.14 .. c:function:: int PyContext_ClearWatcher(int watcher_id) - Clear watcher identified by *watcher_id* previously returned from - :c:func:`PyContext_AddWatcher` for the current interpreter. - Return ``0`` on success, or ``-1`` and set an exception on error - (e.g. if the given *watcher_id* was never registered.) + Clears the watcher identified by *watcher_id* previously returned from + :c:func:`PyContext_AddWatcher` for the current interpreter, and removes the + reference created for the registered callback object. Returns ``0`` on + success, or sets an exception and returns ``-1`` on error (e.g., if the + given *watcher_id* was never registered). .. versionadded:: 3.14 @@ -130,23 +185,6 @@ Context object management functions: .. versionadded:: 3.14 -.. c:type:: int (*PyContext_WatchCallback)(PyContextEvent event, PyObject *obj) - - Context object watcher callback function. The object passed to the callback - is event-specific; see :c:type:`PyContextEvent` for details. - - If the callback returns with an exception set, it must return ``-1``; this - exception will be printed as an unraisable exception using - :c:func:`PyErr_FormatUnraisable`. Otherwise it should return ``0``. - - There may already be a pending exception set on entry to the callback. In - this case, the callback should return ``0`` with the same exception still - set. This means the callback may not call any other API that can set an - exception unless it saves and clears the exception state first, and restores - it before returning. - - .. versionadded:: 3.14 - Context variable functions: diff --git a/Include/cpython/context.h b/Include/cpython/context.h index 3a7a4b459c09ad..4cbdb251b1a0c2 100644 --- a/Include/cpython/context.h +++ b/Include/cpython/context.h @@ -28,37 +28,10 @@ PyAPI_FUNC(int) PyContext_Enter(PyObject *); PyAPI_FUNC(int) PyContext_Exit(PyObject *); typedef enum { - /* - * 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_SWITCHED = 1, } PyContextEvent; -/* - * Context object watcher callback function. The object passed to the callback - * is event-specific; see PyContextEvent for details. - * - * if the callback returns with an exception set, it must return -1. Otherwise - * it should return 0 - */ -typedef int (*PyContext_WatchCallback)(PyContextEvent, PyObject *); - -/* - * Register a per-interpreter callback that will be invoked for context object - * enter/exit events. - * - * Returns a handle that may be passed to PyContext_ClearWatcher on success, - * or -1 and sets and error if no more handles are available. - */ -PyAPI_FUNC(int) PyContext_AddWatcher(PyContext_WatchCallback callback); - -/* - * Clear the watcher associated with the watcher_id handle. - * - * Returns 0 on success or -1 if no watcher exists for the provided id. - */ +PyAPI_FUNC(int) PyContext_AddWatcher(PyObject *callback); PyAPI_FUNC(int) PyContext_ClearWatcher(int watcher_id); /* Create a new context variable. diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 87cdcb5b119d15..27cee228404e50 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -242,7 +242,7 @@ struct _is { PyObject *audit_hooks; PyType_WatchCallback type_watchers[TYPE_MAX_WATCHERS]; PyCode_WatchCallback code_watchers[CODE_MAX_WATCHERS]; - PyContext_WatchCallback context_watchers[CONTEXT_MAX_WATCHERS]; + PyObject *context_watchers[CONTEXT_MAX_WATCHERS]; // One bit is set for each non-NULL entry in code_watchers uint8_t active_code_watchers; uint8_t active_context_watchers; diff --git a/Lib/test/test_capi/test_watchers.py b/Lib/test/test_capi/test_watchers.py index 8644479d83d5ed..fdb47f2f02de68 100644 --- a/Lib/test/test_capi/test_watchers.py +++ b/Lib/test/test_capi/test_watchers.py @@ -1,3 +1,4 @@ +import contextlib import unittest import contextvars @@ -589,60 +590,49 @@ def test_allocate_too_many_watchers(self): class TestContextObjectWatchers(unittest.TestCase): @contextmanager - def context_watcher(self, which_watcher): - wid = _testcapi.add_context_watcher(which_watcher) + def context_watcher(self, cb=None): + log = None + if cb is None: + log = [] + def cb(event, ctx): + self.assertEqual(event, _testcapi.Py_CONTEXT_SWITCHED) + log.append(ctx) + wid = _testcapi.add_context_watcher(cb) try: - switches = _testcapi.get_context_switches(which_watcher) - except ValueError: - switches = None - try: - yield switches + yield log finally: _testcapi.clear_context_watcher(wid) - 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) - - # 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) - 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) - 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(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) - self.assert_event_counts(0, 0) + with self.context_watcher() as switches_0: + self.assertEqual(len(switches_0), 0) + ctx.run(lambda: self.assertEqual(len(switches_0), 1)) + self.assertEqual(len(switches_0), 2) + with self.context_watcher() as switches_1: + self.assertEqual((len(switches_0), len(switches_1)), (2, 0)) + ctx.run(lambda: self.assertEqual( + (len(switches_0), len(switches_1)), (3, 1))) + self.assertEqual((len(switches_0), len(switches_1)), (4, 2)) def test_callback_error(self): ctx_outer = contextvars.copy_context() ctx_inner = contextvars.copy_context() unraisables = [] + def _cb(event, ctx): + raise RuntimeError('boom!') + def _in_outer(): - with self.context_watcher(2): + with self.context_watcher(_cb): with catch_unraisable_exception() as cm: ctx_inner.run(lambda: unraisables.append(cm.unraisable)) unraisables.append(cm.unraisable) try: ctx_outer.run(_in_outer) + self.assertEqual([x is not None for x in unraisables], + [True, True]) self.assertEqual([x.err_msg for x in unraisables], ["Exception ignored in Py_CONTEXT_SWITCHED " f"watcher callback for {ctx!r}" @@ -656,21 +646,24 @@ def _in_outer(): def test_clear_out_of_range_watcher_id(self): with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"): _testcapi.clear_context_watcher(-1) - with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID 8"): - _testcapi.clear_context_watcher(8) # CONTEXT_MAX_WATCHERS = 8 + with self.assertRaisesRegex(ValueError, f"Invalid context watcher ID {_testcapi.CONTEXT_MAX_WATCHERS}"): + _testcapi.clear_context_watcher(_testcapi.CONTEXT_MAX_WATCHERS) def test_clear_unassigned_watcher_id(self): with self.assertRaisesRegex(ValueError, r"No context watcher set for ID 1"): _testcapi.clear_context_watcher(1) def test_allocate_too_many_watchers(self): - with self.assertRaisesRegex(RuntimeError, r"no more context watcher IDs available"): - _testcapi.allocate_too_many_context_watchers() + with contextlib.ExitStack() as stack: + for i in range(_testcapi.CONTEXT_MAX_WATCHERS): + stack.enter_context(self.context_watcher()) + with self.assertRaisesRegex(RuntimeError, r"no more context watcher IDs available"): + stack.enter_context(self.context_watcher()) def test_exit_base_context(self): ctx = contextvars.Context() _testcapi.clear_context_stack() - with self.context_watcher(0) as switches: + with self.context_watcher() as switches: ctx.run(lambda: None) self.assertEqual(switches, [ctx, None]) diff --git a/Misc/NEWS.d/next/C_API/2024-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst b/Misc/NEWS.d/next/C_API/2024-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst new file mode 100644 index 00000000000000..0f51b576441c48 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2024-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst @@ -0,0 +1,2 @@ +Changed :c:func:`PyContext_AddWatcher` to take a callable object instead of a C +function pointer so that the callback can have non-global state. diff --git a/Modules/_testcapi/clinic/watchers.c.h b/Modules/_testcapi/clinic/watchers.c.h index ebd71d119fa347..c4f9797524c885 100644 --- a/Modules/_testcapi/clinic/watchers.c.h +++ b/Modules/_testcapi/clinic/watchers.c.h @@ -132,6 +132,41 @@ _testcapi_unwatch_type(PyObject *module, PyObject *const *args, Py_ssize_t nargs return return_value; } +PyDoc_STRVAR(_testcapi_add_context_watcher__doc__, +"add_context_watcher($module, callback, /)\n" +"--\n" +"\n"); + +#define _TESTCAPI_ADD_CONTEXT_WATCHER_METHODDEF \ + {"add_context_watcher", (PyCFunction)_testcapi_add_context_watcher, METH_O, _testcapi_add_context_watcher__doc__}, + +PyDoc_STRVAR(_testcapi_clear_context_watcher__doc__, +"clear_context_watcher($module, id, /)\n" +"--\n" +"\n"); + +#define _TESTCAPI_CLEAR_CONTEXT_WATCHER_METHODDEF \ + {"clear_context_watcher", (PyCFunction)_testcapi_clear_context_watcher, METH_O, _testcapi_clear_context_watcher__doc__}, + +static PyObject * +_testcapi_clear_context_watcher_impl(PyObject *Py_UNUSED(module), int id); + +static PyObject * +_testcapi_clear_context_watcher(PyObject *module, PyObject *arg) +{ + PyObject *return_value = NULL; + int id; + + id = PyLong_AsInt(arg); + if (id == -1 && PyErr_Occurred()) { + goto exit; + } + return_value = _testcapi_clear_context_watcher_impl(module, id); + +exit: + return return_value; +} + PyDoc_STRVAR(_testcapi_set_func_defaults_via_capi__doc__, "set_func_defaults_via_capi($module, func, defaults, /)\n" "--\n" @@ -191,4 +226,4 @@ _testcapi_set_func_kwdefaults_via_capi(PyObject *module, PyObject *const *args, exit: return return_value; } -/*[clinic end generated code: output=0e07ce7f295917a5 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=0678b313634a92fd input=a9049054013a1b77]*/ diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c index 321d3aeffb6ad1..78fb71af1e391e 100644 --- a/Modules/_testcapi/watchers.c +++ b/Modules/_testcapi/watchers.c @@ -623,88 +623,42 @@ allocate_too_many_func_watchers(PyObject *self, PyObject *args) Py_RETURN_NONE; } -// Test contexct object watchers -#define NUM_CONTEXT_WATCHERS 2 -static int context_watcher_ids[NUM_CONTEXT_WATCHERS] = {-1, -1}; -static PyObject *context_switches[NUM_CONTEXT_WATCHERS]; +// Test context object watchers -static int -handle_context_watcher_event(int which_watcher, PyContextEvent event, PyObject *ctx) { - if (event == Py_CONTEXT_SWITCHED) { - PyList_Append(context_switches[which_watcher], ctx); - } - else { - return -1; - } - return 0; -} - -static int -first_context_watcher_callback(PyContextEvent event, PyObject *ctx) { - return handle_context_watcher_event(0, event, ctx); -} - -static int -second_context_watcher_callback(PyContextEvent event, PyObject *ctx) { - return handle_context_watcher_event(1, event, ctx); -} - -static int -noop_context_event_handler(PyContextEvent event, PyObject *ctx) { - return 0; -} +/*[clinic input] +_testcapi.add_context_watcher + module: self(unused=True) + callback: object + / -static int -error_context_event_handler(PyContextEvent event, PyObject *ctx) { - PyErr_SetString(PyExc_RuntimeError, "boom!"); - return -1; -} +[clinic start generated code]*/ static PyObject * -add_context_watcher(PyObject *self, PyObject *which_watcher) +_testcapi_add_context_watcher(PyObject *Py_UNUSED(module), PyObject *callback) +/*[clinic end generated code: output=ca3710dd0512462c input=58277acf4f4acf24]*/ { - 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 || which_l >= (long)Py_ARRAY_LENGTH(callbacks)) { - PyErr_Format(PyExc_ValueError, "invalid watcher %d", which_l); + int id = PyContext_AddWatcher(callback); + if (id < 0) { 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); + return PyLong_FromLong(id); } +/*[clinic input] +_testcapi.clear_context_watcher + module: self(unused=True) + id: int + / + +[clinic start generated code]*/ + static PyObject * -clear_context_watcher(PyObject *self, PyObject *watcher_id) +_testcapi_clear_context_watcher_impl(PyObject *Py_UNUSED(module), int id) +/*[clinic end generated code: output=2dfa5ce6b55ae2b4 input=b45c860936532813]*/ { - assert(PyLong_Check(watcher_id)); - long watcher_id_l = PyLong_AsLong(watcher_id); - if (PyContext_ClearWatcher(watcher_id_l) < 0) { + if (PyContext_ClearWatcher(id) < 0) { return NULL; } - // reset static events counters - if (watcher_id_l >= 0) { - for (int i = 0; i < NUM_CONTEXT_WATCHERS; i++) { - if (watcher_id_l == context_watcher_ids[i]) { - context_watcher_ids[i] = -1; - Py_CLEAR(context_switches[i]); - } - } - } Py_RETURN_NONE; } @@ -724,51 +678,6 @@ clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } -static PyObject * -get_context_switches(PyObject *Py_UNUSED(self), PyObject *watcher_id) -{ - assert(PyLong_Check(watcher_id)); - long watcher_id_l = PyLong_AsLong(watcher_id); - 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 * -allocate_too_many_context_watchers(PyObject *self, PyObject *args) -{ - int watcher_ids[CONTEXT_MAX_WATCHERS + 1]; - int num_watchers = 0; - for (unsigned long i = 0; i < sizeof(watcher_ids) / sizeof(int); i++) { - int watcher_id = PyContext_AddWatcher(noop_context_event_handler); - if (watcher_id == -1) { - break; - } - watcher_ids[i] = watcher_id; - num_watchers++; - } - PyObject *exc = PyErr_GetRaisedException(); - for (int i = 0; i < num_watchers; i++) { - if (PyContext_ClearWatcher(watcher_ids[i]) < 0) { - PyErr_WriteUnraisable(Py_None); - break; - } - } - if (exc) { - PyErr_SetRaisedException(exc); - return NULL; - } - else if (PyErr_Occurred()) { - return NULL; - } - Py_RETURN_NONE; -} - /*[clinic input] _testcapi.set_func_defaults_via_capi func: object @@ -837,13 +746,10 @@ static PyMethodDef test_methods[] = { {"allocate_too_many_func_watchers", allocate_too_many_func_watchers, METH_NOARGS, NULL}, - // Code object watchers. - {"add_context_watcher", add_context_watcher, METH_O, NULL}, - {"clear_context_watcher", clear_context_watcher, METH_O, NULL}, + // Context object watchers. + _TESTCAPI_ADD_CONTEXT_WATCHER_METHODDEF + _TESTCAPI_CLEAR_CONTEXT_WATCHER_METHODDEF {"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}, }; @@ -854,6 +760,16 @@ _PyTestCapi_Init_Watchers(PyObject *mod) return -1; } +#define ADD_INT_CONST(INT) \ + do { \ + if (PyModule_AddIntConstant(mod, #INT, INT) < 0) { \ + return -1; \ + } \ + } while(0) + ADD_INT_CONST(CONTEXT_MAX_WATCHERS); + ADD_INT_CONST(Py_CONTEXT_SWITCHED); +#undef ADD_INT_CONST + /* Expose each event as an attribute on the module */ #define ADD_EVENT(event) \ if (add_func_event(mod, "PYFUNC_EVENT_" #event, \ diff --git a/Python/context.c b/Python/context.c index 95aa82206270f9..7acf036735a3ec 100644 --- a/Python/context.c +++ b/Python/context.c @@ -125,32 +125,51 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx) assert(interp->_initialized); uint8_t bits = interp->active_context_watchers; int i = 0; + PyObject *args[3] = {NULL, NULL, ctx}; while (bits) { assert(i < CONTEXT_MAX_WATCHERS); if (bits & 1) { - PyContext_WatchCallback cb = interp->context_watchers[i]; + PyObject *cb = interp->context_watchers[i]; assert(cb != NULL); - if (cb(event, ctx) < 0) { + PyObject *exc = _PyErr_GetRaisedException(ts); + if (args[1] == NULL) { + args[1] = PyLong_FromLong(event); + if (args[1] == NULL) { + PyErr_WriteUnraisable(NULL); + _PyErr_SetRaisedException(ts, exc); + return; + } + } + PyObject *ret = PyObject_Vectorcall( + cb, &args[1], 2 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); + if (ret == NULL) { PyErr_FormatUnraisable( "Exception ignored in %s watcher callback for %R", context_event_name(event), ctx); } + Py_CLEAR(ret); + _PyErr_SetRaisedException(ts, exc); } i++; bits >>= 1; } + Py_CLEAR(args[1]); } int -PyContext_AddWatcher(PyContext_WatchCallback callback) +PyContext_AddWatcher(PyObject *callback) { + if (!PyCallable_Check(callback)) { + PyErr_SetString(PyExc_TypeError, "callback is not callable"); + return -1; + } PyInterpreterState *interp = _PyInterpreterState_GET(); assert(interp->_initialized); for (int i = 0; i < CONTEXT_MAX_WATCHERS; i++) { if (!interp->context_watchers[i]) { - interp->context_watchers[i] = callback; + interp->context_watchers[i] = Py_NewRef(callback); interp->active_context_watchers |= (1 << i); return i; } @@ -174,7 +193,7 @@ PyContext_ClearWatcher(int watcher_id) PyErr_Format(PyExc_ValueError, "No context watcher set for ID %d", watcher_id); return -1; } - interp->context_watchers[watcher_id] = NULL; + Py_CLEAR(interp->context_watchers[watcher_id]); interp->active_context_watchers &= ~(1 << watcher_id); return 0; } diff --git a/Python/pystate.c b/Python/pystate.c index 3ceae229f75cd0..7b387a68d9d817 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -902,7 +902,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) interp->active_code_watchers = 0; for (int i=0; i < CONTEXT_MAX_WATCHERS; i++) { - interp->context_watchers[i] = NULL; + Py_CLEAR(interp->context_watchers[i]); } interp->active_context_watchers = 0; // XXX Once we have one allocator per interpreter (i.e. diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 686f3935d91bda..5637a5cef018bb 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -454,9 +454,6 @@ Modules/_testcapi/watchers.c - num_code_object_destroyed_events - 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 - 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 -