From 89ce99cebec99761efdfe95193b6b32271e85338 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 23 Sep 2022 16:58:13 -0400 Subject: [PATCH 01/12] Add an optional callback that is invoked whenever a function is modified JIT compilers may need to invalidate compiled code when a function is modified (e.g. if its code object is modified). This adds the ability to set a callback that, when set, is called each time a function is modified. --- Doc/c-api/function.rst | 53 ++++++ Include/cpython/funcobject.h | 46 +++++ Include/internal/pycore_function.h | 2 + Include/internal/pycore_interp.h | 2 + Lib/test/test_func_events.py | 97 ++++++++++ ...2-10-05-11-44-52.gh-issue-91053.f5Bo3p.rst | 4 + Modules/Setup.stdlib.in | 2 +- Modules/_testcapi/func_events.c | 168 ++++++++++++++++++ Modules/_testcapi/parts.h | 1 + Modules/_testcapimodule.c | 3 + Objects/funcobject.c | 51 ++++++ PCbuild/_testcapi.vcxproj | 3 +- PCbuild/_testcapi.vcxproj.filters | 5 +- Python/pystate.c | 4 + 14 files changed, 438 insertions(+), 3 deletions(-) create mode 100644 Lib/test/test_func_events.py create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-44-52.gh-issue-91053.f5Bo3p.rst create mode 100644 Modules/_testcapi/func_events.c diff --git a/Doc/c-api/function.rst b/Doc/c-api/function.rst index df88e85e518829..a161fb59b91765 100644 --- a/Doc/c-api/function.rst +++ b/Doc/c-api/function.rst @@ -118,3 +118,56 @@ There are a few functions specific to Python functions. must be a dictionary or ``Py_None``. Raises :exc:`SystemError` and returns ``-1`` on failure. + + +.. c:function:: int PyFunction_AddWatcher(PyFunction_WatchCallback callback) + + Register *callback* as a function watcher for the current interpreter. Returns + an id which may be passed to :c:func:`PyFunction_ClearWatcher`. In case + of error (e.g. no more watcher IDs available), return ``-1`` and set an + exception. + + .. versionadded:: 3.12 + + +.. c:function:: int PyFunction_ClearWatcher(int watcher_id) + + Clear watcher identified by *watcher_id* previously returned from + :c:func:`PyFunction_AddWatcher` for the current interpreter. Return ``0`` on + success or ``-1`` on error (e.g. if the given *watcher_id* was never + registered.) + + .. versionadded:: 3.12 + + +.. c:type:: PyFunction_WatchEvent + + Enumeration of possible function watcher events: ``PyFunction_EVENT_CREATED``, + ``PyFunction_EVENT_DESTROY``, ``PyFunction_EVENT_MODIFY_CODE``, + ``PyFunction_EVENT_MODIFY_DEFAULTS``, or ``PyFunction_EVENT_MODIFY_KWDEFAULTS``. + + .. versionadded:: 3.12 + + +.. c:type:: int (*PyFunction_WatchCallback)(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) + + Type of a function watcher callback function. + + If *event* is ``PyFunction_EVENT_CREATED`` or ``PyFunction_EVENT_DESTROY`` + then *new_value* will be ``NULL``. Otherwise, *new_value* will hold a + borrowed reference to the new value that is about to be stored in *func* for + the attribute that is being modified. + + The callback may inspect but must not modify *func*; doing so could have + unpredictable effects, including infinite recursion. + + If *event* is ``PyFunction_EVENT_CREATED``, then the callback is invoked + after `func` has been fully initialized. Otherwise, the callback is invoked + before the modification to *func* takes place, so the prior state of *func* + can be inspected. + + 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_WriteUnraisable`. Otherwise it should return ``0``. + + .. versionadded:: 3.12 diff --git a/Include/cpython/funcobject.h b/Include/cpython/funcobject.h index dd8f20b2c20b39..90582ff0c7616b 100644 --- a/Include/cpython/funcobject.h +++ b/Include/cpython/funcobject.h @@ -131,6 +131,52 @@ PyAPI_DATA(PyTypeObject) PyStaticMethod_Type; PyAPI_FUNC(PyObject *) PyClassMethod_New(PyObject *); PyAPI_FUNC(PyObject *) PyStaticMethod_New(PyObject *); +#define FOREACH_FUNC_EVENT(V) \ + V(CREATED) \ + V(DESTROY) \ + V(MODIFY_CODE) \ + V(MODIFY_DEFAULTS) \ + V(MODIFY_KWDEFAULTS) + +typedef enum { + #define DEF_EVENT(EVENT) PyFunction_EVENT_##EVENT, + FOREACH_FUNC_EVENT(DEF_EVENT) + #undef DEF_EVENT +} PyFunction_WatchEvent; + +/* + * A callback that is invoked for different events in a function's lifecycle. + * + * The callback is invoked with a borrowed reference to func, after it is + * created and before it is modified or destroyed. The callback should not + * modify func. + * + * When a function's code object, defaults, or kwdefaults are modified the + * callback will be invoked with the respective event and new_value will + * contain a borrowed reference to the new value that is about to be stored in + * the function. Otherwise the third argument is NULL. + */ +typedef int (*PyFunction_WatchCallback)( + PyFunction_WatchEvent event, + PyFunctionObject *func, + PyObject *new_value); + +/* + * Register a per-interpreter callback that will be invoked for function lifecycle + * events. + * + * Returns a handle that may be passed to PyFunction_ClearWatcher on success, + * or -1 and sets an error if no more handles are available. + */ +PyAPI_FUNC(int) PyFunction_AddWatcher(PyFunction_WatchCallback callback); + +/* + * Clear the watcher associated with the watcher_id handle. + * + * Returns 0 on success or -1 if no watcher exists for the supplied id. + */ +PyAPI_FUNC(int) PyFunction_ClearWatcher(int watcher_id); + #ifdef __cplusplus } #endif diff --git a/Include/internal/pycore_function.h b/Include/internal/pycore_function.h index 1c87aa31ddb611..9d4928e4a30bec 100644 --- a/Include/internal/pycore_function.h +++ b/Include/internal/pycore_function.h @@ -8,6 +8,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +#define FUNC_MAX_WATCHERS 8 + extern PyFunctionObject* _PyFunction_FromConstructor(PyFrameConstructor *constr); extern uint32_t _PyFunction_GetVersionForCurrentState(PyFunctionObject *func); diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index c11e897305d42b..da9cbd8333436e 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -17,6 +17,7 @@ extern "C" { #include "pycore_dict.h" // struct _Py_dict_state #include "pycore_exceptions.h" // struct _Py_exc_state #include "pycore_floatobject.h" // struct _Py_float_state +#include "pycore_function.h" // FUNC_MAX_WATCHERS #include "pycore_genobject.h" // struct _Py_async_gen_state #include "pycore_gc.h" // struct _gc_runtime_state #include "pycore_list.h" // struct _Py_list_state @@ -147,6 +148,7 @@ struct _is { _PyFrameEvalFunction eval_frame; PyDict_WatchCallback dict_watchers[DICT_MAX_WATCHERS]; + PyFunction_WatchCallback func_watchers[FUNC_MAX_WATCHERS]; Py_ssize_t co_extra_user_count; freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; diff --git a/Lib/test/test_func_events.py b/Lib/test/test_func_events.py new file mode 100644 index 00000000000000..c709c3d2830710 --- /dev/null +++ b/Lib/test/test_func_events.py @@ -0,0 +1,97 @@ +import unittest + +from contextlib import contextmanager +from test.support import catch_unraisable_exception, import_helper + +_testcapi = import_helper.import_module("_testcapi") + +from _testcapi import ( + PYFUNC_EVENT_CREATED, + PYFUNC_EVENT_DESTROY, + PYFUNC_EVENT_MODIFY_CODE, + PYFUNC_EVENT_MODIFY_DEFAULTS, + PYFUNC_EVENT_MODIFY_KWDEFAULTS, + _add_func_watcher, + _clear_func_watcher, +) + + +class FuncEventsTest(unittest.TestCase): + @contextmanager + def add_watcher(self, func): + wid = _add_func_watcher(func) + try: + yield + finally: + _clear_func_watcher(wid) + + def test_func_events_dispatched(self): + events = [] + def watcher(*args): + events.append(args) + + with self.add_watcher(watcher): + def myfunc(): + pass + self.assertIn((PYFUNC_EVENT_CREATED, myfunc, None), events) + myfunc_id = id(myfunc) + + new_code = self.test_func_events_dispatched.__code__ + myfunc.__code__ = new_code + self.assertIn((PYFUNC_EVENT_MODIFY_CODE, myfunc, new_code), events) + + new_defaults = (123,) + myfunc.__defaults__ = new_defaults + self.assertIn((PYFUNC_EVENT_MODIFY_DEFAULTS, myfunc, new_defaults), events) + + new_kwdefaults = {"self": 123} + myfunc.__kwdefaults__ = new_kwdefaults + self.assertIn((PYFUNC_EVENT_MODIFY_KWDEFAULTS, myfunc, new_kwdefaults), events) + + # Clear events reference to func + events = [] + del myfunc + self.assertIn((PYFUNC_EVENT_DESTROY, myfunc_id, None), events) + + def test_multiple_watchers(self): + events0 = [] + def first_watcher(*args): + events0.append(args) + + events1 = [] + def second_watcher(*args): + events1.append(args) + + with self.add_watcher(first_watcher): + with self.add_watcher(second_watcher): + def myfunc(): + pass + + event = (PYFUNC_EVENT_CREATED, myfunc, None) + self.assertIn(event, events0) + self.assertIn(event, events1) + + def test_watcher_raises_error(self): + class MyError(Exception): + pass + + def watcher(*args): + raise MyError("testing 123") + + with self.add_watcher(watcher): + with catch_unraisable_exception() as cm: + def myfunc(): + pass + + self.assertIs(cm.unraisable.object, myfunc) + self.assertIsInstance(cm.unraisable.exc_value, MyError) + + def test_clear_out_of_range_watcher_id(self): + with self.assertRaisesRegex(ValueError, r"Invalid func watcher ID -1"): + _clear_func_watcher(-1) + with self.assertRaisesRegex(ValueError, r"Invalid func watcher ID 8"): + _clear_func_watcher(8) # FUNC_MAX_WATCHERS = 8 + + def test_clear_unassigned_watcher_id(self): + with self.assertRaisesRegex(ValueError, r"No func watcher set for ID 1"): + _clear_func_watcher(1) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-44-52.gh-issue-91053.f5Bo3p.rst b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-44-52.gh-issue-91053.f5Bo3p.rst new file mode 100644 index 00000000000000..59bb12caef740d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-44-52.gh-issue-91053.f5Bo3p.rst @@ -0,0 +1,4 @@ +Optimizing interpreters and JIT compilers may need to invalidate internal +metadata when functions are modified. This change adds the ability to +provide a callback that will be invoked each time a function is created, +modified, or destroyed. diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index ac8959ebea5bf2..2003ede9076f36 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -169,7 +169,7 @@ @MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c -@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c +@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/func_events.c # Some testing modules MUST be built as shared libraries. *shared* diff --git a/Modules/_testcapi/func_events.c b/Modules/_testcapi/func_events.c new file mode 100644 index 00000000000000..994722e8d13e95 --- /dev/null +++ b/Modules/_testcapi/func_events.c @@ -0,0 +1,168 @@ +#include "parts.h" + +#define NUM_WATCHERS 2 +static PyObject *pyfunc_watchers[NUM_WATCHERS]; +static int watcher_ids[NUM_WATCHERS] = {-1, -1}; + +static PyObject *get_id(PyObject *obj) { + PyObject *builtins = PyEval_GetBuiltins(); + if (builtins == NULL) { + return NULL; + } + PyObject *id_str = PyUnicode_FromString("id"); + if (id_str == NULL) { + return NULL; + } + PyObject *id_func = PyObject_GetItem(builtins, id_str); + Py_DECREF(id_str); + if (id_func == NULL) { + return NULL; + } + PyObject *stack[] = {obj}; + PyObject *id = PyObject_Vectorcall(id_func, stack, 1, NULL); + Py_DECREF(id_func); + return id; +} + +static int +call_pyfunc_watcher(PyObject *watcher, PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +{ + PyObject *event_obj = PyLong_FromLong(event); + if (event_obj == NULL) { + return -1; + } + if (new_value == NULL) { + new_value = Py_None; + } + Py_INCREF(new_value); + PyObject *func_or_id = NULL; + if (event == PyFunction_EVENT_DESTROY) { + /* Don't expose a function that's about to be destroyed to managed code */ + func_or_id = get_id((PyObject *) func); + if (func_or_id == NULL) { + Py_DECREF(event_obj); + Py_DECREF(new_value); + return -1; + } + } else { + Py_INCREF(func); + func_or_id = (PyObject *) func; + } + PyObject *stack[] = {event_obj, func_or_id, new_value}; + PyObject *res = PyObject_Vectorcall(watcher, stack, 3, NULL); + int st = (res == NULL) ? -1 : 0; + Py_XDECREF(res); + Py_DECREF(new_value); + Py_DECREF(event_obj); + Py_DECREF(func_or_id); + return st; +} + +static int +first_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +{ + return call_pyfunc_watcher(pyfunc_watchers[0], event, func, new_value); +} + +static int +second_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +{ + return call_pyfunc_watcher(pyfunc_watchers[1], event, func, new_value); +} + +static PyFunction_WatchCallback watcher_callbacks[NUM_WATCHERS] = { + first_watcher_callback, + second_watcher_callback +}; + +static int +add_event(PyObject *module, const char *name, PyFunction_WatchEvent event) +{ + PyObject *value = PyLong_FromLong(event); + if (value == NULL) { + return -1; + } + int ok = PyModule_AddObjectRef(module, name, value); + Py_DECREF(value); + return ok; +} + +static PyObject * +add_watcher(PyObject *self, PyObject *func) +{ + if (!PyFunction_Check(func)) { + PyErr_SetString(PyExc_TypeError, "'func' must be a function"); + return NULL; + } + int idx = -1; + for (int i = 0; i < NUM_WATCHERS; i++) { + if (watcher_ids[i] == -1) { + idx = i; + break; + } + } + if (idx == -1) { + PyErr_SetString(PyExc_RuntimeError, "no free watchers"); + return NULL; + } + PyObject *result = PyLong_FromLong(idx); + if (result == NULL) { + return NULL; + } + watcher_ids[idx] = PyFunction_AddWatcher(watcher_callbacks[idx]); + if (watcher_ids[idx] < 0) { + Py_DECREF(result); + return NULL; + } + pyfunc_watchers[idx] = func; + Py_INCREF(func); + return result; +} + +static PyObject * +clear_watcher(PyObject *self, PyObject *watcher_id_obj) +{ + long watcher_id = PyLong_AsLong(watcher_id_obj); + if ((watcher_id < INT_MIN) || (watcher_id > INT_MAX)) { + PyErr_SetString(PyExc_ValueError, "invalid watcher id"); + return NULL; + } + int wid = (int) watcher_id; + if (PyFunction_ClearWatcher(wid) < 0) { + return NULL; + } + int idx = -1; + for (int i = 0; i < NUM_WATCHERS; i++) { + if (watcher_ids[i] == wid) { + idx = i; + break; + } + } + assert(idx != -1); + Py_CLEAR(pyfunc_watchers[idx]); + watcher_ids[idx] = -1; + Py_RETURN_NONE; +} + +static PyMethodDef TestMethods[] = { + {"_add_func_watcher", add_watcher, METH_O}, + {"_clear_func_watcher", clear_watcher, METH_O}, + {NULL}, +}; + +int +_PyTestCapi_Init_FuncEvents(PyObject *m) { + if (PyModule_AddFunctions(m, TestMethods) < 0) { + return -1; + } + + /* Expose each event as an attribute on the module */ +#define ADD_EVENT(event) \ + if (add_event(m, "PYFUNC_EVENT_" #event, PyFunction_EVENT_##event)) { \ + return -1; \ + } + FOREACH_FUNC_EVENT(ADD_EVENT); +#undef ADD_EVENT + + return 0; +} diff --git a/Modules/_testcapi/parts.h b/Modules/_testcapi/parts.h index 304e5922c0d50b..9d998aaa5a5a03 100644 --- a/Modules/_testcapi/parts.h +++ b/Modules/_testcapi/parts.h @@ -27,6 +27,7 @@ int _PyTestCapi_Init_Vectorcall(PyObject *module); int _PyTestCapi_Init_Heaptype(PyObject *module); int _PyTestCapi_Init_Unicode(PyObject *module); +int _PyTestCapi_Init_FuncEvents(PyObject *module); #ifdef LIMITED_API_AVAILABLE int _PyTestCapi_Init_VectorcallLimited(PyObject *module); diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 95c67fc95e891a..59ed1618baf928 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -6751,6 +6751,9 @@ PyInit__testcapi(void) if (_PyTestCapi_Init_Unicode(m) < 0) { return NULL; } + if (_PyTestCapi_Init_FuncEvents(m) < 0) { + return NULL; + } #ifndef LIMITED_API_AVAILABLE PyModule_AddObjectRef(m, "LIMITED_API_AVAILABLE", Py_False); diff --git a/Objects/funcobject.c b/Objects/funcobject.c index ccc6d0b52eab68..c19b08ac9748be 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -3,12 +3,57 @@ #include "Python.h" #include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals() +#include "pycore_function.h" // FUNC_MAX_WATCHERS #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_pyerrors.h" // _PyErr_Occurred() #include "structmember.h" // PyMemberDef static uint32_t next_func_version = 1; +static void +handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + for (int i = 0; i < FUNC_MAX_WATCHERS; i++) { + PyFunction_WatchCallback cb = interp->func_watchers[i]; + if ((cb != NULL) && (cb(event, func, new_value) < 0)) { + PyErr_WriteUnraisable((PyObject *) func); + } + } +} + +int +PyFunction_AddWatcher(PyFunction_WatchCallback callback) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + assert(interp->_initialized); + for (int i = 0; i < FUNC_MAX_WATCHERS; i++) { + if (interp->func_watchers[i] == NULL) { + interp->func_watchers[i] = callback; + return i; + } + } + PyErr_SetString(PyExc_RuntimeError, "no more func watcher IDs available"); + return -1; + +} + +int +PyFunction_ClearWatcher(int watcher_id) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (watcher_id < 0 || watcher_id >= FUNC_MAX_WATCHERS) { + PyErr_Format(PyExc_ValueError, "Invalid func watcher ID %d", watcher_id); + return -1; + } + if (!interp->func_watchers[watcher_id]) { + PyErr_Format(PyExc_ValueError, "No func watcher set for ID %d", watcher_id); + return -1; + } + interp->func_watchers[watcher_id] = NULL; + return 0; +} + PyFunctionObject * _PyFunction_FromConstructor(PyFrameConstructor *constr) { @@ -40,6 +85,7 @@ _PyFunction_FromConstructor(PyFrameConstructor *constr) op->vectorcall = _PyFunction_Vectorcall; op->func_version = 0; _PyObject_GC_TRACK(op); + handle_func_event(PyFunction_EVENT_CREATED, op, NULL); return op; } @@ -116,6 +162,7 @@ PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname op->vectorcall = _PyFunction_Vectorcall; op->func_version = 0; _PyObject_GC_TRACK(op); + handle_func_event(PyFunction_EVENT_CREATED, op, NULL); return (PyObject *)op; error: @@ -401,6 +448,7 @@ func_set_code(PyFunctionObject *op, PyObject *value, void *Py_UNUSED(ignored)) nclosure, nfree); return -1; } + handle_func_event(PyFunction_EVENT_MODIFY_CODE, op, value); op->func_version = 0; Py_INCREF(value); Py_XSETREF(op->func_code, value); @@ -486,6 +534,7 @@ func_set_defaults(PyFunctionObject *op, PyObject *value, void *Py_UNUSED(ignored return -1; } + handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, op, value); op->func_version = 0; Py_XINCREF(value); Py_XSETREF(op->func_defaults, value); @@ -528,6 +577,7 @@ func_set_kwdefaults(PyFunctionObject *op, PyObject *value, void *Py_UNUSED(ignor return -1; } + handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, op, value); op->func_version = 0; Py_XINCREF(value); Py_XSETREF(op->func_kwdefaults, value); @@ -715,6 +765,7 @@ func_clear(PyFunctionObject *op) static void func_dealloc(PyFunctionObject *op) { + handle_func_event(PyFunction_EVENT_DESTROY, op, NULL); _PyObject_GC_UNTRACK(op); if (op->func_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject *) op); diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index b7d40c8cdc30eb..8a408095462993 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -98,6 +98,7 @@ + @@ -115,4 +116,4 @@ - \ No newline at end of file + diff --git a/PCbuild/_testcapi.vcxproj.filters b/PCbuild/_testcapi.vcxproj.filters index fc2c4345fe142e..12361a78990525 100644 --- a/PCbuild/_testcapi.vcxproj.filters +++ b/PCbuild/_testcapi.vcxproj.filters @@ -24,10 +24,13 @@ Source Files + + Source Files + Resource Files - \ No newline at end of file + diff --git a/Python/pystate.c b/Python/pystate.c index c74868ddfa20f3..e94a72b6287f4f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -455,6 +455,10 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) interp->dict_watchers[i] = NULL; } + for (int i=0; i < FUNC_MAX_WATCHERS; i++) { + interp->func_watchers[i] = NULL; + } + // XXX Once we have one allocator per interpreter (i.e. // per-interpreter GC) we must ensure that all of the interpreter's // objects have been cleaned up at the point. From 4d4823cb206ed628e8ef4de9ca9d0d144a16f4f2 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 12:26:48 -0800 Subject: [PATCH 02/12] Use imperative mood for all enum values --- Include/cpython/funcobject.h | 2 +- Lib/test/test_func_events.py | 6 +++--- Objects/funcobject.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Include/cpython/funcobject.h b/Include/cpython/funcobject.h index 90582ff0c7616b..953694cedd54d3 100644 --- a/Include/cpython/funcobject.h +++ b/Include/cpython/funcobject.h @@ -132,7 +132,7 @@ PyAPI_FUNC(PyObject *) PyClassMethod_New(PyObject *); PyAPI_FUNC(PyObject *) PyStaticMethod_New(PyObject *); #define FOREACH_FUNC_EVENT(V) \ - V(CREATED) \ + V(CREATE) \ V(DESTROY) \ V(MODIFY_CODE) \ V(MODIFY_DEFAULTS) \ diff --git a/Lib/test/test_func_events.py b/Lib/test/test_func_events.py index c709c3d2830710..fb9bfabf414735 100644 --- a/Lib/test/test_func_events.py +++ b/Lib/test/test_func_events.py @@ -6,7 +6,7 @@ _testcapi = import_helper.import_module("_testcapi") from _testcapi import ( - PYFUNC_EVENT_CREATED, + PYFUNC_EVENT_CREATE, PYFUNC_EVENT_DESTROY, PYFUNC_EVENT_MODIFY_CODE, PYFUNC_EVENT_MODIFY_DEFAULTS, @@ -33,7 +33,7 @@ def watcher(*args): with self.add_watcher(watcher): def myfunc(): pass - self.assertIn((PYFUNC_EVENT_CREATED, myfunc, None), events) + self.assertIn((PYFUNC_EVENT_CREATE, myfunc, None), events) myfunc_id = id(myfunc) new_code = self.test_func_events_dispatched.__code__ @@ -67,7 +67,7 @@ def second_watcher(*args): def myfunc(): pass - event = (PYFUNC_EVENT_CREATED, myfunc, None) + event = (PYFUNC_EVENT_CREATE, myfunc, None) self.assertIn(event, events0) self.assertIn(event, events1) diff --git a/Objects/funcobject.c b/Objects/funcobject.c index c19b08ac9748be..814c840cdada6a 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -85,7 +85,7 @@ _PyFunction_FromConstructor(PyFrameConstructor *constr) op->vectorcall = _PyFunction_Vectorcall; op->func_version = 0; _PyObject_GC_TRACK(op); - handle_func_event(PyFunction_EVENT_CREATED, op, NULL); + handle_func_event(PyFunction_EVENT_CREATE, op, NULL); return op; } @@ -162,7 +162,7 @@ PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname op->vectorcall = _PyFunction_Vectorcall; op->func_version = 0; _PyObject_GC_TRACK(op); - handle_func_event(PyFunction_EVENT_CREATED, op, NULL); + handle_func_event(PyFunction_EVENT_CREATE, op, NULL); return (PyObject *)op; error: From acde67e4952492691ed800815ea3dc722722171b Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 12:27:38 -0800 Subject: [PATCH 03/12] Update docs - Imperative mood - Semantic line breaks --- Doc/c-api/function.rst | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/Doc/c-api/function.rst b/Doc/c-api/function.rst index a161fb59b91765..377650207448a3 100644 --- a/Doc/c-api/function.rst +++ b/Doc/c-api/function.rst @@ -122,10 +122,10 @@ There are a few functions specific to Python functions. .. c:function:: int PyFunction_AddWatcher(PyFunction_WatchCallback callback) - Register *callback* as a function watcher for the current interpreter. Returns - an id which may be passed to :c:func:`PyFunction_ClearWatcher`. In case - of error (e.g. no more watcher IDs available), return ``-1`` and set an - exception. + Register *callback* as a function watcher for the current interpreter. + Return an ID which may be passed to :c:func:`PyFunction_ClearWatcher`. + In case of error (e.g. no more watcher IDs available), + return ``-1`` and set an exception. .. versionadded:: 3.12 @@ -133,18 +133,21 @@ There are a few functions specific to Python functions. .. c:function:: int PyFunction_ClearWatcher(int watcher_id) Clear watcher identified by *watcher_id* previously returned from - :c:func:`PyFunction_AddWatcher` for the current interpreter. Return ``0`` on - success or ``-1`` on error (e.g. if the given *watcher_id* was never - registered.) + :c:func:`PyFunction_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.) .. versionadded:: 3.12 .. c:type:: PyFunction_WatchEvent - Enumeration of possible function watcher events: ``PyFunction_EVENT_CREATED``, - ``PyFunction_EVENT_DESTROY``, ``PyFunction_EVENT_MODIFY_CODE``, - ``PyFunction_EVENT_MODIFY_DEFAULTS``, or ``PyFunction_EVENT_MODIFY_KWDEFAULTS``. + Enumeration of possible function watcher events: + - ``PyFunction_EVENT_CREATE`` + - ``PyFunction_EVENT_DESTROY`` + - ``PyFunction_EVENT_MODIFY_CODE`` + - ``PyFunction_EVENT_MODIFY_DEFAULTS`` + - ``PyFunction_EVENT_MODIFY_KWDEFAULTS`` .. versionadded:: 3.12 @@ -153,15 +156,15 @@ There are a few functions specific to Python functions. Type of a function watcher callback function. - If *event* is ``PyFunction_EVENT_CREATED`` or ``PyFunction_EVENT_DESTROY`` + If *event* is ``PyFunction_EVENT_CREATE`` or ``PyFunction_EVENT_DESTROY`` then *new_value* will be ``NULL``. Otherwise, *new_value* will hold a - borrowed reference to the new value that is about to be stored in *func* for - the attribute that is being modified. + :term:`borrowed reference` to the new value that is about to be stored in + *func* for the attribute that is being modified. The callback may inspect but must not modify *func*; doing so could have unpredictable effects, including infinite recursion. - If *event* is ``PyFunction_EVENT_CREATED``, then the callback is invoked + If *event* is ``PyFunction_EVENT_CREATE``, then the callback is invoked after `func` has been fully initialized. Otherwise, the callback is invoked before the modification to *func* takes place, so the prior state of *func* can be inspected. From 896d70879b2ade0f47026adc7293b40c1dcd30bc Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 12:42:58 -0800 Subject: [PATCH 04/12] Conform to PEP-7 --- Lib/test/test_func_events.py | 6 +++--- Modules/_testcapi/func_events.c | 38 +++++++++++++++++++-------------- Objects/funcobject.c | 7 +++--- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_func_events.py b/Lib/test/test_func_events.py index fb9bfabf414735..6114516c53688e 100644 --- a/Lib/test/test_func_events.py +++ b/Lib/test/test_func_events.py @@ -87,11 +87,11 @@ def myfunc(): self.assertIsInstance(cm.unraisable.exc_value, MyError) def test_clear_out_of_range_watcher_id(self): - with self.assertRaisesRegex(ValueError, r"Invalid func watcher ID -1"): + with self.assertRaisesRegex(ValueError, r"invalid func watcher ID -1"): _clear_func_watcher(-1) - with self.assertRaisesRegex(ValueError, r"Invalid func watcher ID 8"): + with self.assertRaisesRegex(ValueError, r"invalid func watcher ID 8"): _clear_func_watcher(8) # FUNC_MAX_WATCHERS = 8 def test_clear_unassigned_watcher_id(self): - with self.assertRaisesRegex(ValueError, r"No func watcher set for ID 1"): + with self.assertRaisesRegex(ValueError, r"no func watcher set for ID 1"): _clear_func_watcher(1) diff --git a/Modules/_testcapi/func_events.c b/Modules/_testcapi/func_events.c index 994722e8d13e95..3fa6bbe965daa2 100644 --- a/Modules/_testcapi/func_events.c +++ b/Modules/_testcapi/func_events.c @@ -4,8 +4,10 @@ static PyObject *pyfunc_watchers[NUM_WATCHERS]; static int watcher_ids[NUM_WATCHERS] = {-1, -1}; -static PyObject *get_id(PyObject *obj) { - PyObject *builtins = PyEval_GetBuiltins(); +static +PyObject *get_id(PyObject *obj) +{ + PyObject *builtins = PyEval_GetBuiltins(); // borrowed ref. if (builtins == NULL) { return NULL; } @@ -25,7 +27,8 @@ static PyObject *get_id(PyObject *obj) { } static int -call_pyfunc_watcher(PyObject *watcher, PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +call_pyfunc_watcher(PyObject *watcher, PyFunction_WatchEvent event, + PyFunctionObject *func, PyObject *new_value) { PyObject *event_obj = PyLong_FromLong(event); if (event_obj == NULL) { @@ -44,7 +47,8 @@ call_pyfunc_watcher(PyObject *watcher, PyFunction_WatchEvent event, PyFunctionOb Py_DECREF(new_value); return -1; } - } else { + } + else { Py_INCREF(func); func_or_id = (PyObject *) func; } @@ -59,20 +63,22 @@ call_pyfunc_watcher(PyObject *watcher, PyFunction_WatchEvent event, PyFunctionOb } static int -first_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +first_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, + PyObject *new_value) { return call_pyfunc_watcher(pyfunc_watchers[0], event, func, new_value); } static int -second_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +second_watcher_callback(PyFunction_WatchEvent event, PyFunctionObject *func, + PyObject *new_value) { return call_pyfunc_watcher(pyfunc_watchers[1], event, func, new_value); } static PyFunction_WatchCallback watcher_callbacks[NUM_WATCHERS] = { - first_watcher_callback, - second_watcher_callback + first_watcher_callback, + second_watcher_callback }; static int @@ -114,8 +120,7 @@ add_watcher(PyObject *self, PyObject *func) Py_DECREF(result); return NULL; } - pyfunc_watchers[idx] = func; - Py_INCREF(func); + pyfunc_watchers[idx] = Py_NewRef(func); return result; } @@ -124,7 +129,7 @@ clear_watcher(PyObject *self, PyObject *watcher_id_obj) { long watcher_id = PyLong_AsLong(watcher_id_obj); if ((watcher_id < INT_MIN) || (watcher_id > INT_MAX)) { - PyErr_SetString(PyExc_ValueError, "invalid watcher id"); + PyErr_SetString(PyExc_ValueError, "invalid watcher ID"); return NULL; } int wid = (int) watcher_id; @@ -133,10 +138,10 @@ clear_watcher(PyObject *self, PyObject *watcher_id_obj) } int idx = -1; for (int i = 0; i < NUM_WATCHERS; i++) { - if (watcher_ids[i] == wid) { - idx = i; - break; - } + if (watcher_ids[i] == wid) { + idx = i; + break; + } } assert(idx != -1); Py_CLEAR(pyfunc_watchers[idx]); @@ -151,7 +156,8 @@ static PyMethodDef TestMethods[] = { }; int -_PyTestCapi_Init_FuncEvents(PyObject *m) { +_PyTestCapi_Init_FuncEvents(PyObject *m) +{ if (PyModule_AddFunctions(m, TestMethods) < 0) { return -1; } diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 814c840cdada6a..f964f941a34a36 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -35,7 +35,6 @@ PyFunction_AddWatcher(PyFunction_WatchCallback callback) } PyErr_SetString(PyExc_RuntimeError, "no more func watcher IDs available"); return -1; - } int @@ -43,11 +42,13 @@ PyFunction_ClearWatcher(int watcher_id) { PyInterpreterState *interp = _PyInterpreterState_GET(); if (watcher_id < 0 || watcher_id >= FUNC_MAX_WATCHERS) { - PyErr_Format(PyExc_ValueError, "Invalid func watcher ID %d", watcher_id); + PyErr_Format(PyExc_ValueError, "invalid func watcher ID %d", + watcher_id); return -1; } if (!interp->func_watchers[watcher_id]) { - PyErr_Format(PyExc_ValueError, "No func watcher set for ID %d", watcher_id); + PyErr_Format(PyExc_ValueError, "no func watcher set for ID %d", + watcher_id); return -1; } interp->func_watchers[watcher_id] = NULL; From 788ad349c9b9544661e4d2f41f6a9a5d9c739ad1 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 14:02:44 -0800 Subject: [PATCH 05/12] Move tests into test_capi --- Lib/test/test_capi.py | 91 +++++++++++++++++++++++++++++++++ Lib/test/test_func_events.py | 97 ------------------------------------ 2 files changed, 91 insertions(+), 97 deletions(-) delete mode 100644 Lib/test/test_func_events.py diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 17425050ce00c0..53bcbb8ecaa04b 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -37,6 +37,16 @@ # Skip this test if the _testcapi module isn't available. _testcapi = import_helper.import_module('_testcapi') +from _testcapi import ( + PYFUNC_EVENT_CREATE, + PYFUNC_EVENT_DESTROY, + PYFUNC_EVENT_MODIFY_CODE, + PYFUNC_EVENT_MODIFY_DEFAULTS, + PYFUNC_EVENT_MODIFY_KWDEFAULTS, + _add_func_watcher, + _clear_func_watcher, +) + import _testinternalcapi @@ -1556,5 +1566,86 @@ def test_clear_unassigned_watcher_id(self): self.clear_watcher(1) +class FuncEventsTest(unittest.TestCase): + @contextmanager + def add_watcher(self, func): + wid = _add_func_watcher(func) + try: + yield + finally: + _clear_func_watcher(wid) + + def test_func_events_dispatched(self): + events = [] + def watcher(*args): + events.append(args) + + with self.add_watcher(watcher): + def myfunc(): + pass + self.assertIn((PYFUNC_EVENT_CREATE, myfunc, None), events) + myfunc_id = id(myfunc) + + new_code = self.test_func_events_dispatched.__code__ + myfunc.__code__ = new_code + self.assertIn((PYFUNC_EVENT_MODIFY_CODE, myfunc, new_code), events) + + new_defaults = (123,) + myfunc.__defaults__ = new_defaults + self.assertIn((PYFUNC_EVENT_MODIFY_DEFAULTS, myfunc, new_defaults), events) + + new_kwdefaults = {"self": 123} + myfunc.__kwdefaults__ = new_kwdefaults + self.assertIn((PYFUNC_EVENT_MODIFY_KWDEFAULTS, myfunc, new_kwdefaults), events) + + # Clear events reference to func + events = [] + del myfunc + self.assertIn((PYFUNC_EVENT_DESTROY, myfunc_id, None), events) + + def test_multiple_watchers(self): + events0 = [] + def first_watcher(*args): + events0.append(args) + + events1 = [] + def second_watcher(*args): + events1.append(args) + + with self.add_watcher(first_watcher): + with self.add_watcher(second_watcher): + def myfunc(): + pass + + event = (PYFUNC_EVENT_CREATE, myfunc, None) + self.assertIn(event, events0) + self.assertIn(event, events1) + + def test_watcher_raises_error(self): + class MyError(Exception): + pass + + def watcher(*args): + raise MyError("testing 123") + + with self.add_watcher(watcher): + with catch_unraisable_exception() as cm: + def myfunc(): + pass + + self.assertIs(cm.unraisable.object, myfunc) + self.assertIsInstance(cm.unraisable.exc_value, MyError) + + def test_clear_out_of_range_watcher_id(self): + with self.assertRaisesRegex(ValueError, r"invalid func watcher ID -1"): + _clear_func_watcher(-1) + with self.assertRaisesRegex(ValueError, r"invalid func watcher ID 8"): + _clear_func_watcher(8) # FUNC_MAX_WATCHERS = 8 + + def test_clear_unassigned_watcher_id(self): + with self.assertRaisesRegex(ValueError, r"no func watcher set for ID 1"): + _clear_func_watcher(1) + + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_func_events.py b/Lib/test/test_func_events.py deleted file mode 100644 index 6114516c53688e..00000000000000 --- a/Lib/test/test_func_events.py +++ /dev/null @@ -1,97 +0,0 @@ -import unittest - -from contextlib import contextmanager -from test.support import catch_unraisable_exception, import_helper - -_testcapi = import_helper.import_module("_testcapi") - -from _testcapi import ( - PYFUNC_EVENT_CREATE, - PYFUNC_EVENT_DESTROY, - PYFUNC_EVENT_MODIFY_CODE, - PYFUNC_EVENT_MODIFY_DEFAULTS, - PYFUNC_EVENT_MODIFY_KWDEFAULTS, - _add_func_watcher, - _clear_func_watcher, -) - - -class FuncEventsTest(unittest.TestCase): - @contextmanager - def add_watcher(self, func): - wid = _add_func_watcher(func) - try: - yield - finally: - _clear_func_watcher(wid) - - def test_func_events_dispatched(self): - events = [] - def watcher(*args): - events.append(args) - - with self.add_watcher(watcher): - def myfunc(): - pass - self.assertIn((PYFUNC_EVENT_CREATE, myfunc, None), events) - myfunc_id = id(myfunc) - - new_code = self.test_func_events_dispatched.__code__ - myfunc.__code__ = new_code - self.assertIn((PYFUNC_EVENT_MODIFY_CODE, myfunc, new_code), events) - - new_defaults = (123,) - myfunc.__defaults__ = new_defaults - self.assertIn((PYFUNC_EVENT_MODIFY_DEFAULTS, myfunc, new_defaults), events) - - new_kwdefaults = {"self": 123} - myfunc.__kwdefaults__ = new_kwdefaults - self.assertIn((PYFUNC_EVENT_MODIFY_KWDEFAULTS, myfunc, new_kwdefaults), events) - - # Clear events reference to func - events = [] - del myfunc - self.assertIn((PYFUNC_EVENT_DESTROY, myfunc_id, None), events) - - def test_multiple_watchers(self): - events0 = [] - def first_watcher(*args): - events0.append(args) - - events1 = [] - def second_watcher(*args): - events1.append(args) - - with self.add_watcher(first_watcher): - with self.add_watcher(second_watcher): - def myfunc(): - pass - - event = (PYFUNC_EVENT_CREATE, myfunc, None) - self.assertIn(event, events0) - self.assertIn(event, events1) - - def test_watcher_raises_error(self): - class MyError(Exception): - pass - - def watcher(*args): - raise MyError("testing 123") - - with self.add_watcher(watcher): - with catch_unraisable_exception() as cm: - def myfunc(): - pass - - self.assertIs(cm.unraisable.object, myfunc) - self.assertIsInstance(cm.unraisable.exc_value, MyError) - - def test_clear_out_of_range_watcher_id(self): - with self.assertRaisesRegex(ValueError, r"invalid func watcher ID -1"): - _clear_func_watcher(-1) - with self.assertRaisesRegex(ValueError, r"invalid func watcher ID 8"): - _clear_func_watcher(8) # FUNC_MAX_WATCHERS = 8 - - def test_clear_unassigned_watcher_id(self): - with self.assertRaisesRegex(ValueError, r"no func watcher set for ID 1"): - _clear_func_watcher(1) From c1895282e7ea2c3548acd3f257be1a4c8e0eed3b Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 15:20:39 -0800 Subject: [PATCH 06/12] Add a test for allocating too many watchers --- Include/cpython/funcobject.h | 3 +++ Lib/test/test_capi.py | 5 ++++ Modules/_testcapi/func_events.c | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/Include/cpython/funcobject.h b/Include/cpython/funcobject.h index 953694cedd54d3..5979febc2e3421 100644 --- a/Include/cpython/funcobject.h +++ b/Include/cpython/funcobject.h @@ -155,6 +155,9 @@ typedef enum { * callback will be invoked with the respective event and new_value will * contain a borrowed reference to the new value that is about to be stored in * the function. Otherwise the third argument is NULL. + * + * If the callback returns with an exception set, it must return -1. Otherwise + * it should return 0. */ typedef int (*PyFunction_WatchCallback)( PyFunction_WatchEvent event, diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 53bcbb8ecaa04b..833755f2d8e62b 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -44,6 +44,7 @@ PYFUNC_EVENT_MODIFY_DEFAULTS, PYFUNC_EVENT_MODIFY_KWDEFAULTS, _add_func_watcher, + _allocate_too_many_func_watchers, _clear_func_watcher, ) @@ -1646,6 +1647,10 @@ def test_clear_unassigned_watcher_id(self): with self.assertRaisesRegex(ValueError, r"no func watcher set for ID 1"): _clear_func_watcher(1) + def test_allocate_too_many_watchers(self): + with self.assertRaisesRegex(RuntimeError, r"no more func watcher IDs"): + _allocate_too_many_func_watchers() + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapi/func_events.c b/Modules/_testcapi/func_events.c index 3fa6bbe965daa2..ccd743efe7be8f 100644 --- a/Modules/_testcapi/func_events.c +++ b/Modules/_testcapi/func_events.c @@ -1,5 +1,8 @@ #include "parts.h" +#define Py_BUILD_CORE +#include "pycore_function.h" // FUNC_MAX_WATCHERS + #define NUM_WATCHERS 2 static PyObject *pyfunc_watchers[NUM_WATCHERS]; static int watcher_ids[NUM_WATCHERS] = {-1, -1}; @@ -149,9 +152,48 @@ clear_watcher(PyObject *self, PyObject *watcher_id_obj) Py_RETURN_NONE; } +static int +noop_handler(PyFunction_WatchEvent event, PyFunctionObject *func, + PyObject *new_value) +{ + return 0; +} + +static PyObject * +allocate_too_many_watchers(PyObject *self, PyObject *args) +{ + int watcher_ids[FUNC_MAX_WATCHERS + 1]; + int num_watchers = 0; + for (unsigned long i = 0; i < sizeof(watcher_ids) / sizeof(int); i++) { + int watcher_id = PyFunction_AddWatcher(noop_handler); + if (watcher_id == -1) { + break; + } + watcher_ids[i] = watcher_id; + num_watchers++; + } + PyObject *type, *value, *traceback; + PyErr_Fetch(&type, &value, &traceback); + for (int i = 0; i < num_watchers; i++) { + if (PyFunction_ClearWatcher(watcher_ids[i]) < 0) { + PyErr_WriteUnraisable(Py_None); + break; + } + } + if (type) { + PyErr_Restore(type, value, traceback); + return NULL; + } + else if (PyErr_Occurred()) { + return NULL; + } + Py_RETURN_NONE; +} + static PyMethodDef TestMethods[] = { {"_add_func_watcher", add_watcher, METH_O}, {"_clear_func_watcher", clear_watcher, METH_O}, + {"_allocate_too_many_func_watchers", allocate_too_many_watchers, METH_NOARGS}, {NULL}, }; From 9d3496ff6efd61532fc43366edf82d2cef4b0e9e Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 12:56:50 -0800 Subject: [PATCH 07/12] Dispatch events when {kw}defaults are modified via the C-API --- Objects/funcobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/funcobject.c b/Objects/funcobject.c index f964f941a34a36..3147e85056fa99 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -255,6 +255,7 @@ PyFunction_SetDefaults(PyObject *op, PyObject *defaults) PyErr_SetString(PyExc_SystemError, "non-tuple default args"); return -1; } + handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, op, NULL); ((PyFunctionObject *)op)->func_version = 0; Py_XSETREF(((PyFunctionObject *)op)->func_defaults, defaults); return 0; @@ -295,6 +296,7 @@ PyFunction_SetKwDefaults(PyObject *op, PyObject *defaults) "non-dict keyword only default args"); return -1; } + handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, op, NULL); ((PyFunctionObject *)op)->func_version = 0; Py_XSETREF(((PyFunctionObject *)op)->func_kwdefaults, defaults); return 0; From 0fdc7d774861f7dc51bb954ec80f5c0081b9aa29 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 8 Nov 2022 16:12:20 -0800 Subject: [PATCH 08/12] Trigger events when {kw}defaults are changed via the C-API --- Lib/test/test_capi.py | 10 ++++++++++ Modules/_testcapi/func_events.c | 30 ++++++++++++++++++++++++++++++ Objects/funcobject.c | 6 ++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 833755f2d8e62b..84c4efbfc695c7 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -46,6 +46,8 @@ _add_func_watcher, _allocate_too_many_func_watchers, _clear_func_watcher, + _set_func_defaults_via_capi, + _set_func_kwdefaults_via_capi, ) import _testinternalcapi @@ -1595,10 +1597,18 @@ def myfunc(): myfunc.__defaults__ = new_defaults self.assertIn((PYFUNC_EVENT_MODIFY_DEFAULTS, myfunc, new_defaults), events) + new_defaults = (456,) + _set_func_defaults_via_capi(myfunc, new_defaults) + self.assertIn((PYFUNC_EVENT_MODIFY_DEFAULTS, myfunc, new_defaults), events) + new_kwdefaults = {"self": 123} myfunc.__kwdefaults__ = new_kwdefaults self.assertIn((PYFUNC_EVENT_MODIFY_KWDEFAULTS, myfunc, new_kwdefaults), events) + new_kwdefaults = {"self": 456} + _set_func_kwdefaults_via_capi(myfunc, new_kwdefaults) + self.assertIn((PYFUNC_EVENT_MODIFY_KWDEFAULTS, myfunc, new_kwdefaults), events) + # Clear events reference to func events = [] del myfunc diff --git a/Modules/_testcapi/func_events.c b/Modules/_testcapi/func_events.c index ccd743efe7be8f..054f382a6a1e7c 100644 --- a/Modules/_testcapi/func_events.c +++ b/Modules/_testcapi/func_events.c @@ -190,10 +190,40 @@ allocate_too_many_watchers(PyObject *self, PyObject *args) Py_RETURN_NONE; } +static PyObject * +set_defaults(PyObject *self, PyObject *args) +{ + PyObject *func = NULL; + PyObject *defaults = NULL; + if (!PyArg_ParseTuple(args, "OO", &func, &defaults)) { + return NULL; + } + if (PyFunction_SetDefaults(func, defaults) < 0) { + return NULL; + } + Py_RETURN_NONE; +} + +static PyObject * +set_kwdefaults(PyObject *self, PyObject *args) +{ + PyObject *func = NULL; + PyObject *kwdefaults = NULL; + if (!PyArg_ParseTuple(args, "OO", &func, &kwdefaults)) { + return NULL; + } + if (PyFunction_SetKwDefaults(func, kwdefaults) < 0) { + return NULL; + } + Py_RETURN_NONE; +} + static PyMethodDef TestMethods[] = { {"_add_func_watcher", add_watcher, METH_O}, {"_clear_func_watcher", clear_watcher, METH_O}, {"_allocate_too_many_func_watchers", allocate_too_many_watchers, METH_NOARGS}, + {"_set_func_defaults_via_capi", set_defaults, METH_VARARGS}, + {"_set_func_kwdefaults_via_capi", set_kwdefaults, METH_VARARGS}, {NULL}, }; diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 3147e85056fa99..0eee45468c8ddf 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -255,7 +255,8 @@ PyFunction_SetDefaults(PyObject *op, PyObject *defaults) PyErr_SetString(PyExc_SystemError, "non-tuple default args"); return -1; } - handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, op, NULL); + handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, + (PyFunctionObject *) op, defaults); ((PyFunctionObject *)op)->func_version = 0; Py_XSETREF(((PyFunctionObject *)op)->func_defaults, defaults); return 0; @@ -296,7 +297,8 @@ PyFunction_SetKwDefaults(PyObject *op, PyObject *defaults) "non-dict keyword only default args"); return -1; } - handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, op, NULL); + handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, + (PyFunctionObject *) op, defaults); ((PyFunctionObject *)op)->func_version = 0; Py_XSETREF(((PyFunctionObject *)op)->func_kwdefaults, defaults); return 0; From a79e6188fba072a83af6a595d6df2ce31d9c0dec Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 9 Nov 2022 09:43:11 -0800 Subject: [PATCH 09/12] More PEP-7 --- Modules/_testcapi/func_events.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_testcapi/func_events.c b/Modules/_testcapi/func_events.c index 054f382a6a1e7c..e7cbba79cb0e7d 100644 --- a/Modules/_testcapi/func_events.c +++ b/Modules/_testcapi/func_events.c @@ -7,8 +7,8 @@ static PyObject *pyfunc_watchers[NUM_WATCHERS]; static int watcher_ids[NUM_WATCHERS] = {-1, -1}; -static -PyObject *get_id(PyObject *obj) +static PyObject * +get_id(PyObject *obj) { PyObject *builtins = PyEval_GetBuiltins(); // borrowed ref. if (builtins == NULL) { From dd41468ea8109dd8f2a643a4c7abb81b04d80250 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 21 Nov 2022 20:47:09 -0800 Subject: [PATCH 10/12] Document that the runtime is free to optimize away function creation --- Doc/c-api/function.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Doc/c-api/function.rst b/Doc/c-api/function.rst index 377650207448a3..3cce18bdde3057 100644 --- a/Doc/c-api/function.rst +++ b/Doc/c-api/function.rst @@ -167,7 +167,11 @@ There are a few functions specific to Python functions. If *event* is ``PyFunction_EVENT_CREATE``, then the callback is invoked after `func` has been fully initialized. Otherwise, the callback is invoked before the modification to *func* takes place, so the prior state of *func* - can be inspected. + can be inspected. The runtime is permitted to optimize away the creation of + function objects when possible. In such cases no event will be emitted. + Although this creates the possitibility of an observable difference of + runtime behavior depending on optimization decisions, it does not change + the semantics of the Python code being executed. If the callback returns with an exception set, it must return ``-1``; this exception will be printed as an unraisable exception using From 812dd5fa42ad2f3d6820838a4e18ea9dfc94bff2 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 21 Nov 2022 20:48:05 -0800 Subject: [PATCH 11/12] Add a bit vector to optimize watcher dispatch A bit is set in the bit vector iff there is a watcher set at the corresponding offset in the watcher array. Only notify watchers if at least one bit is set. --- Include/internal/pycore_interp.h | 2 ++ Objects/funcobject.c | 16 ++++++++++++++-- Python/pystate.c | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 732e96fd22505a..532b28499080f2 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -174,6 +174,8 @@ struct _is { PyDict_WatchCallback dict_watchers[DICT_MAX_WATCHERS]; PyFunction_WatchCallback func_watchers[FUNC_MAX_WATCHERS]; + // One bit is set for each non-NULL entry in func_watchers + uint8_t active_func_watchers; Py_ssize_t co_extra_user_count; freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 1f298121cf01a1..bf97edc53ad7d9 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -9,9 +9,9 @@ #include "structmember.h" // PyMemberDef static void -handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject *new_value) +notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event, + PyFunctionObject *func, PyObject *new_value) { - PyInterpreterState *interp = _PyInterpreterState_GET(); for (int i = 0; i < FUNC_MAX_WATCHERS; i++) { PyFunction_WatchCallback cb = interp->func_watchers[i]; if ((cb != NULL) && (cb(event, func, new_value) < 0)) { @@ -20,6 +20,16 @@ handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func, PyObject } } +static inline void +handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func, + PyObject *new_value) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->active_func_watchers) { + notify_func_watchers(interp, event, func, new_value); + } +} + int PyFunction_AddWatcher(PyFunction_WatchCallback callback) { @@ -28,6 +38,7 @@ PyFunction_AddWatcher(PyFunction_WatchCallback callback) for (int i = 0; i < FUNC_MAX_WATCHERS; i++) { if (interp->func_watchers[i] == NULL) { interp->func_watchers[i] = callback; + interp->active_func_watchers |= (1 << i); return i; } } @@ -50,6 +61,7 @@ PyFunction_ClearWatcher(int watcher_id) return -1; } interp->func_watchers[watcher_id] = NULL; + interp->active_func_watchers &= ~(1 << watcher_id); return 0; } diff --git a/Python/pystate.c b/Python/pystate.c index c9faf3b4f932e8..19fd9a6ae4497b 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -464,6 +464,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) for (int i=0; i < FUNC_MAX_WATCHERS; i++) { interp->func_watchers[i] = NULL; } + interp->active_func_watchers = 0; // XXX Once we have one allocator per interpreter (i.e. // per-interpreter GC) we must ensure that all of the interpreter's From e7b3325a8dbd458e0db573cee9117050f7bde4be Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 22 Nov 2022 10:15:40 +0100 Subject: [PATCH 12/12] Revert added newlines to VS project files --- PCbuild/_testcapi.vcxproj | 2 +- PCbuild/_testcapi.vcxproj.filters | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index fe9a8d26109ccd..58bf4e1eacbf21 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -124,4 +124,4 @@ - + \ No newline at end of file diff --git a/PCbuild/_testcapi.vcxproj.filters b/PCbuild/_testcapi.vcxproj.filters index 82d611cb458113..101c5322761634 100644 --- a/PCbuild/_testcapi.vcxproj.filters +++ b/PCbuild/_testcapi.vcxproj.filters @@ -57,4 +57,4 @@ Resource Files - + \ No newline at end of file