From c9e95d40c9c15194b04b098e846fe2b9eae922ea Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 22 Nov 2024 00:41:18 -0500 Subject: [PATCH 1/3] gh-127124: Pass optional state to context watcher callback This enables users to associate state with the callback without relying on globals. Also: * Refactor the tests for improved readability and extensibility, and to cover the new state object. * Drop the pointer from the `PyContext_WatchCallback` typedef. This de-obfuscates the fact that pointers are involved, and makes it possible to forward-declare functions to improve readability: static PyContext_WatchCallback my_callback; int my_callback(PyObject *cbarg, PyContextEvent event, PyObject *obj) { ... } --- Doc/c-api/contextvars.rst | 30 ++-- Include/cpython/context.h | 27 +--- Include/internal/pycore_interp.h | 5 +- Lib/test/test_capi/test_watchers.py | 55 +++----- ...-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst | 2 + Modules/_testcapi/clinic/watchers.c.h | 37 ++++- Modules/_testcapi/watchers.c | 130 +++++++----------- Python/context.c | 17 ++- Python/pystate.c | 3 +- Tools/c-analyzer/cpython/ignored.tsv | 3 - 10 files changed, 145 insertions(+), 164 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2024-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst diff --git a/Doc/c-api/contextvars.rst b/Doc/c-api/contextvars.rst index b7c6550ff34aac..df7b9fa14652d7 100644 --- a/Doc/c-api/contextvars.rst +++ b/Doc/c-api/contextvars.rst @@ -101,21 +101,25 @@ 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) +.. c:function:: int PyContext_AddWatcher(PyContext_WatchCallback *callback, PyObject *cbarg) - 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. + Registers *callback* as a context object watcher for the current + interpreter, and *cbarg* (which may be NULL; if not, this function creates a + new reference) as the object to pass as the callback's first parameter. On + success, returns a non-negative ID which may be passed to + :c:func:`PyContext_ClearWatcher` to unregister the callback and remove the + added reference to *cbarg*. Sets an exception and returns ``-1`` on error + (e.g., no more watcher IDs available). .. 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 *cbarg* object that was registered with the + callback. 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,10 +134,12 @@ Context object management functions: .. versionadded:: 3.14 -.. c:type:: int (*PyContext_WatchCallback)(PyContextEvent event, PyObject *obj) +.. c:type:: int PyContext_WatchCallback(PyObject *cbarg, PyContextEvent event, PyObject *obj) - Context object watcher callback function. The object passed to the callback - is event-specific; see :c:type:`PyContextEvent` for details. + Context object watcher callback function. *cbarg* is the same object + registered in the call to :c:func:`PyContext_AddWatcher`, as a borrowed + reference if non-NULL. The *obj* object 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 diff --git a/Include/cpython/context.h b/Include/cpython/context.h index 3a7a4b459c09ad..bcf63de5fdb30e 100644 --- a/Include/cpython/context.h +++ b/Include/cpython/context.h @@ -36,29 +36,10 @@ typedef enum { 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. - */ +typedef int PyContext_WatchCallback( + PyObject *cbarg, PyContextEvent event, PyObject *obj); +PyAPI_FUNC(int) PyContext_AddWatcher( + PyContext_WatchCallback *callback, PyObject *cbarg); 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..5a08e0fc6ec79c 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -242,7 +242,10 @@ 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]; + struct { + PyContext_WatchCallback *callback; + PyObject *arg; + } 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..7cd2937a13355d 100644 --- a/Lib/test/test_capi/test_watchers.py +++ b/Lib/test/test_capi/test_watchers.py @@ -589,46 +589,27 @@ 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, cfg=None): + if cfg is None: + cfg = {} + cfg.setdefault('log', []) + wid = _testcapi.add_context_watcher(cfg) try: - switches = _testcapi.get_context_switches(which_watcher) - except ValueError: - switches = None - try: - yield switches + yield cfg.get('log', None) 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() @@ -636,13 +617,15 @@ def test_callback_error(self): unraisables = [] def _in_outer(): - with self.context_watcher(2): + with self.context_watcher(cfg={'err': RuntimeError('boom!')}): 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}" @@ -670,7 +653,7 @@ def test_allocate_too_many_watchers(self): 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..51eb6f438356ec --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2024-11-22-18-38-33.gh-issue-127124.6k6Qj7.rst @@ -0,0 +1,2 @@ +Added optional callback state to :c:func:`PyContext_AddWatcher` and +:c:type:`PyContext_WatchCallback`. diff --git a/Modules/_testcapi/clinic/watchers.c.h b/Modules/_testcapi/clinic/watchers.c.h index ebd71d119fa347..d9bae9d82b4d1c 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, cfg, /)\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=800450c2d40bebd6 input=a9049054013a1b77]*/ diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c index 321d3aeffb6ad1..5df47f0101786d 100644 --- a/Modules/_testcapi/watchers.c +++ b/Modules/_testcapi/watchers.c @@ -623,88 +623,74 @@ 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); +context_watcher(PyObject *cfg, PyContextEvent event, PyObject *ctx) +{ + if (event != Py_CONTEXT_SWITCHED) { + Py_UNREACHABLE(); } - else { + if (cfg == NULL) { + return 0; // no-op + } + assert(PyDict_Check(cfg)); + PyObject *log = PyDict_GetItemString(cfg, "log"); + if (log != NULL) { + assert(PyList_Check(log)); + if (PyList_Append(log, ctx) < 0) { + Py_UNREACHABLE(); + } + } + PyObject *err = NULL; + if (PyDict_GetItemStringRef(cfg, "err", &err) < 0) { + return -1; + } + if (err != NULL) { + PyErr_SetRaisedException(err); 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) + cfg: 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 *cfg) +/*[clinic end generated code: output=5bd58aed114e0696 input=d49d40dfe0be4930]*/ { - 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); + if (!PyDict_Check(cfg)) { + PyErr_SetString(PyExc_TypeError, "cfg is not a dict"); return NULL; } - int watcher_id = PyContext_AddWatcher(callbacks[which_l]); - if (watcher_id < 0) { + int id = PyContext_AddWatcher(&context_watcher, cfg); + if (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,28 +710,13 @@ 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); + int watcher_id = PyContext_AddWatcher(&context_watcher, NULL); if (watcher_id == -1) { break; } @@ -837,11 +808,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}, diff --git a/Python/context.c b/Python/context.c index 95aa82206270f9..dc4694ed955f6e 100644 --- a/Python/context.c +++ b/Python/context.c @@ -128,9 +128,10 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx) while (bits) { assert(i < CONTEXT_MAX_WATCHERS); if (bits & 1) { - PyContext_WatchCallback cb = interp->context_watchers[i]; + PyContext_WatchCallback *cb = interp->context_watchers[i].callback; + PyObject *arg = interp->context_watchers[i].arg; assert(cb != NULL); - if (cb(event, ctx) < 0) { + if (cb(arg, event, ctx) < 0) { PyErr_FormatUnraisable( "Exception ignored in %s watcher callback for %R", context_event_name(event), ctx); @@ -143,14 +144,15 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx) int -PyContext_AddWatcher(PyContext_WatchCallback callback) +PyContext_AddWatcher(PyContext_WatchCallback *callback, PyObject *arg) { 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; + if (!interp->context_watchers[i].callback) { + interp->context_watchers[i].callback = callback; + Py_XSETREF(interp->context_watchers[i].arg, Py_XNewRef(arg)); interp->active_context_watchers |= (1 << i); return i; } @@ -170,11 +172,12 @@ PyContext_ClearWatcher(int watcher_id) PyErr_Format(PyExc_ValueError, "Invalid context watcher ID %d", watcher_id); return -1; } - if (!interp->context_watchers[watcher_id]) { + if (!interp->context_watchers[watcher_id].callback) { PyErr_Format(PyExc_ValueError, "No context watcher set for ID %d", watcher_id); return -1; } - interp->context_watchers[watcher_id] = NULL; + interp->context_watchers[watcher_id].callback = NULL; + Py_CLEAR(interp->context_watchers[watcher_id].arg); interp->active_context_watchers &= ~(1 << watcher_id); return 0; } diff --git a/Python/pystate.c b/Python/pystate.c index 3ceae229f75cd0..1ccfe5079241c9 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -902,7 +902,8 @@ 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; + interp->context_watchers[i].callback = NULL; + Py_CLEAR(interp->context_watchers[i].arg); } 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 - From 133e00c08d488b297eb30f9833b2569cb08cd2d3 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 25 Nov 2024 04:07:44 -0500 Subject: [PATCH 2/3] fixup! gh-127124: Pass optional state to context watcher callback --- Lib/test/test_capi/test_watchers.py | 12 ++++++--- Modules/_testcapi/watchers.c | 42 +++++++---------------------- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/Lib/test/test_capi/test_watchers.py b/Lib/test/test_capi/test_watchers.py index 7cd2937a13355d..b6dfa0b2dbe964 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 @@ -639,16 +640,19 @@ 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() diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c index 5df47f0101786d..e577242f5084ec 100644 --- a/Modules/_testcapi/watchers.c +++ b/Modules/_testcapi/watchers.c @@ -710,36 +710,6 @@ clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } -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(&context_watcher, NULL); - 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 @@ -812,8 +782,6 @@ static PyMethodDef test_methods[] = { _TESTCAPI_ADD_CONTEXT_WATCHER_METHODDEF _TESTCAPI_CLEAR_CONTEXT_WATCHER_METHODDEF {"clear_context_stack", clear_context_stack, METH_NOARGS, NULL}, - {"allocate_too_many_context_watchers", - (PyCFunction) allocate_too_many_context_watchers, METH_NOARGS, NULL}, {NULL}, }; @@ -824,6 +792,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, \ From 3e28b5fa6af7add4676339119612fceba0598741 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 26 Nov 2024 03:19:54 -0500 Subject: [PATCH 3/3] fixup! gh-127124: Change context watcher callback to a callable object --- Include/cpython/context.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Include/cpython/context.h b/Include/cpython/context.h index bcf63de5fdb30e..e6f183f0fa8f62 100644 --- a/Include/cpython/context.h +++ b/Include/cpython/context.h @@ -28,11 +28,6 @@ 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;