Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

gh-102381: don't call watcher callback with dead object (#102382) 

Co-authored-by: T. Wouters <thomas@python.org>
This commit is contained in:
Carl Meyer 2023-03-07 17:10:58 -07:00 committed by GitHub
parent a33ca2ad1f
commit 1e703a4733
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 243 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
Doc/c-api/code.rst
View File

@ -172,6 +172,11 @@ bound into a function.
before the destruction of *co* takes place, so the prior state of *co*
can be inspected.
If *event* is ``PY_CODE_EVENT_DESTROY``, taking a reference in the callback
to the about-to-be-destroyed code object will resurrect it and prevent it
from being freed at this time. When the resurrected object is destroyed
later, any watcher callbacks active at that time will be called again.
Users of this API should not rely on internal runtime implementation
details. Such details may include, but are not limited to, the exact
order and timing of creation and destruction of code objects. While
@ -179,9 +184,15 @@ bound into a function.
(including whether a callback is invoked or not), 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
:c:func:`PyErr_WriteUnraisable`. Otherwise it should return ``0``.
If the callback sets an exception, it must return ``-1``; this exception will
be printed as an unraisable exception using :c:func:`PyErr_WriteUnraisable`.
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.12

21
Doc/c-api/dict.rst
View File

@ -298,13 +298,26 @@ Dictionary Objects
dictionary.
The callback may inspect but must not modify *dict*; doing so could have
unpredictable effects, including infinite recursion.
unpredictable effects, including infinite recursion. Do not trigger Python
code execution in the callback, as it could modify the dict as a side effect.
If *event* is ``PyDict_EVENT_DEALLOCATED``, taking a new reference in the
callback to the about-to-be-destroyed dictionary will resurrect it and
prevent it from being freed at this time. When the resurrected object is
destroyed later, any watcher callbacks active at that time will be called
again.
Callbacks occur before the notified modification to *dict* takes place, so
the prior state of *dict* 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``.
If the callback sets an exception, it must return ``-1``; this exception will
be printed as an unraisable exception using :c:func:`PyErr_WriteUnraisable`.
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.12

17
Doc/c-api/function.rst
View File

@ -173,8 +173,19 @@ There are a few functions specific to Python functions.
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
:c:func:`PyErr_WriteUnraisable`. Otherwise it should return ``0``.
If *event* is ``PyFunction_EVENT_DESTROY``, Taking a reference in the
callback to the about-to-be-destroyed function will resurrect it, preventing
it from being freed at this time. When the resurrected object is destroyed
later, any watcher callbacks active at that time will be called again.
If the callback sets an exception, it must return ``-1``; this exception will
be printed as an unraisable exception using :c:func:`PyErr_WriteUnraisable`.
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.12

13
Include/cpython/code.h
View File

@ -224,9 +224,14 @@ PyAPI_FUNC(int) PyCode_Addr2Line(PyCodeObject *, int);
PyAPI_FUNC(int) PyCode_Addr2Location(PyCodeObject *, int, int *, int *, int *, int *);
typedef enum PyCodeEvent {
PY_CODE_EVENT_CREATE,
PY_CODE_EVENT_DESTROY
#define PY_FOREACH_CODE_EVENT(V) \
V(CREATE) \
V(DESTROY)
typedef enum {
#define PY_DEF_EVENT(op) PY_CODE_EVENT_##op,
PY_FOREACH_CODE_EVENT(PY_DEF_EVENT)
#undef PY_DEF_EVENT
} PyCodeEvent;
@ -236,7 +241,7 @@ typedef enum PyCodeEvent {
* The callback is invoked with a borrowed reference to co, after it is
* created and before it is destroyed.
*
* If the callback returns with an exception set, it must return -1. Otherwise
* If the callback sets an exception, it must return -1. Otherwise
* it should return 0.
*/
typedef int (*PyCode_WatchCallback)(

17
Include/cpython/dictobject.h
View File

@ -90,13 +90,18 @@ PyAPI_FUNC(PyObject *) _PyDictView_Intersect(PyObject* self, PyObject *other);
/* Dictionary watchers */
#define PY_FOREACH_DICT_EVENT(V) \
V(ADDED) \
V(MODIFIED) \
V(DELETED) \
V(CLONED) \
V(CLEARED) \
V(DEALLOCATED)
typedef enum {
PyDict_EVENT_ADDED,
PyDict_EVENT_MODIFIED,
PyDict_EVENT_DELETED,
PyDict_EVENT_CLONED,
PyDict_EVENT_CLEARED,
PyDict_EVENT_DEALLOCATED,
#define PY_DEF_EVENT(EVENT) PyDict_EVENT_##EVENT,
PY_FOREACH_DICT_EVENT(PY_DEF_EVENT)
#undef PY_DEF_EVENT
} PyDict_WatchEvent;
// Callback to be invoked when a watched dict is cleared, dealloced, or modified.

8
Include/cpython/funcobject.h
View File

@ -131,7 +131,7 @@ PyAPI_DATA(PyTypeObject) PyStaticMethod_Type;
PyAPI_FUNC(PyObject *) PyClassMethod_New(PyObject *);
PyAPI_FUNC(PyObject *) PyStaticMethod_New(PyObject *);
#define FOREACH_FUNC_EVENT(V) \
#define PY_FOREACH_FUNC_EVENT(V) \
V(CREATE) \
V(DESTROY) \
V(MODIFY_CODE) \
@ -139,9 +139,9 @@ PyAPI_FUNC(PyObject *) PyStaticMethod_New(PyObject *);
V(MODIFY_KWDEFAULTS)
typedef enum {
#define DEF_EVENT(EVENT) PyFunction_EVENT_##EVENT,
FOREACH_FUNC_EVENT(DEF_EVENT)
#undef DEF_EVENT
#define PY_DEF_EVENT(EVENT) PyFunction_EVENT_##EVENT,
PY_FOREACH_FUNC_EVENT(PY_DEF_EVENT)
#undef PY_DEF_EVENT
} PyFunction_WatchEvent;
/*

1
Include/internal/pycore_dict.h
View File

@ -164,6 +164,7 @@ _PyDict_NotifyEvent(PyDict_WatchEvent event,
PyObject *key,
PyObject *value)
{
assert(Py_REFCNT((PyObject*)mp) > 0);
int watcher_bits = mp->ma_version_tag & DICT_VERSION_MASK;
if (watcher_bits) {
_PyDict_SendEvent(watcher_bits, event, mp, key, value);

52
Lib/test/test_capi/test_watchers.py
View File

@ -109,10 +109,21 @@ class TestDictWatchers(unittest.TestCase):
self.watch(wid, d)
with catch_unraisable_exception() as cm:
d["foo"] = "bar"
self.assertIs(cm.unraisable.object, d)
self.assertIn(
"PyDict_EVENT_ADDED watcher callback for <dict at",
cm.unraisable.object
)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
self.assert_events([])
def test_dealloc_error(self):
d = {}
with self.watcher(kind=self.ERROR) as wid:
self.watch(wid, d)
with catch_unraisable_exception() as cm:
del d
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
def test_two_watchers(self):
d1 = {}
d2 = {}
@ -389,6 +400,25 @@ class TestCodeObjectWatchers(unittest.TestCase):
del co4
self.assert_event_counts(0, 0, 0, 0)
def test_error(self):
with self.code_watcher(2):
with catch_unraisable_exception() as cm:
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
self.assertEqual(
cm.unraisable.object,
f"PY_CODE_EVENT_CREATE watcher callback for {co!r}"
)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
def test_dealloc_error(self):
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
with self.code_watcher(2):
with catch_unraisable_exception() as cm:
del co
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
def test_clear_out_of_range_watcher_id(self):
with self.assertRaisesRegex(ValueError, r"Invalid code watcher ID -1"):
_testcapi.clear_code_watcher(-1)
@ -479,7 +509,25 @@ class TestFuncWatchers(unittest.TestCase):
def myfunc():
pass
self.assertIs(cm.unraisable.object, myfunc)
self.assertEqual(
cm.unraisable.object,
f"PyFunction_EVENT_CREATE watcher callback for {myfunc!r}"
)
def test_dealloc_watcher_raises_error(self):
class MyError(Exception):
pass
def watcher(*args):
raise MyError("testing 123")
def myfunc():
pass
with self.add_watcher(watcher):
with catch_unraisable_exception() as cm:
del myfunc
self.assertIsInstance(cm.unraisable.exc_value, MyError)
def test_clear_out_of_range_watcher_id(self):

13
Modules/_testcapi/watchers.c
View File

@ -317,6 +317,13 @@ noop_code_event_handler(PyCodeEvent event, PyCodeObject *co)
return 0;
}
static int
error_code_event_handler(PyCodeEvent event, PyCodeObject *co)
{
PyErr_SetString(PyExc_RuntimeError, "boom!");
return -1;
}
static PyObject *
add_code_watcher(PyObject *self, PyObject *which_watcher)
{
@ -333,7 +340,11 @@ add_code_watcher(PyObject *self, PyObject *which_watcher)
num_code_object_created_events[1] = 0;
num_code_object_destroyed_events[1] = 0;
}
else if (which_l == 2) {
watcher_id = PyCode_AddWatcher(error_code_event_handler);
}
else {
PyErr_Format(PyExc_ValueError, "invalid watcher %d", which_l);
return NULL;
}
if (watcher_id < 0) {
@ -672,7 +683,7 @@ _PyTestCapi_Init_Watchers(PyObject *mod)
PyFunction_EVENT_##event)) { \
return -1; \
}
FOREACH_FUNC_EVENT(ADD_EVENT);
PY_FOREACH_FUNC_EVENT(ADD_EVENT);
#undef ADD_EVENT
return 0;

38
Objects/codeobject.c
View File

@ -11,9 +11,24 @@
#include "pycore_tuple.h" // _PyTuple_ITEMS()
#include "clinic/codeobject.c.h"
static PyObject* code_repr(PyCodeObject *co);
static const char *
code_event_name(PyCodeEvent event) {
switch (event) {
#define CASE(op) \
case PY_CODE_EVENT_##op: \
return "PY_CODE_EVENT_" #op;
PY_FOREACH_CODE_EVENT(CASE)
#undef CASE
}
Py_UNREACHABLE();
}
static void
notify_code_watchers(PyCodeEvent event, PyCodeObject *co)
{
assert(Py_REFCNT(co) > 0);
PyInterpreterState *interp = _PyInterpreterState_GET();
assert(interp->_initialized);
uint8_t bits = interp->active_code_watchers;
@ -25,7 +40,21 @@ notify_code_watchers(PyCodeEvent event, PyCodeObject *co)
// callback must be non-null if the watcher bit is set
assert(cb != NULL);
if (cb(event, co) < 0) {
PyErr_WriteUnraisable((PyObject *) co);
// Don't risk resurrecting the object if an unraisablehook keeps
// a reference; pass a string as context.
PyObject *context = NULL;
PyObject *repr = code_repr(co);
if (repr) {
context = PyUnicode_FromFormat(
"%s watcher callback for %U",
code_event_name(event), repr);
Py_DECREF(repr);
}
if (context == NULL) {
context = Py_NewRef(Py_None);
}
PyErr_WriteUnraisable(context);
Py_DECREF(context);
}
}
i++;
@ -1667,7 +1696,14 @@ code_new_impl(PyTypeObject *type, int argcount, int posonlyargcount,
static void
code_dealloc(PyCodeObject *co)
{
assert(Py_REFCNT(co) == 0);
Py_SET_REFCNT(co, 1);
notify_code_watchers(PY_CODE_EVENT_DESTROY, co);
if (Py_REFCNT(co) > 1) {
Py_SET_REFCNT(co, Py_REFCNT(co) - 1);
return;
}
Py_SET_REFCNT(co, 0);
if (co->co_extra != NULL) {
PyInterpreterState *interp = _PyInterpreterState_GET();

34
Objects/dictobject.c
View File

@ -2308,7 +2308,14 @@ Fail:
static void
dict_dealloc(PyDictObject *mp)
{
assert(Py_REFCNT(mp) == 0);
Py_SET_REFCNT(mp, 1);
_PyDict_NotifyEvent(PyDict_EVENT_DEALLOCATED, mp, NULL, NULL);
if (Py_REFCNT(mp) > 1) {
Py_SET_REFCNT(mp, Py_REFCNT(mp) - 1);
return;
}
Py_SET_REFCNT(mp, 0);
PyDictValues *values = mp->ma_values;
PyDictKeysObject *keys = mp->ma_keys;
Py_ssize_t i, n;
@ -5732,6 +5739,18 @@ PyDict_ClearWatcher(int watcher_id)
return 0;
}
static const char *
dict_event_name(PyDict_WatchEvent event) {
switch (event) {
#define CASE(op) \
case PyDict_EVENT_##op: \
return "PyDict_EVENT_" #op;
PY_FOREACH_DICT_EVENT(CASE)
#undef CASE
}
Py_UNREACHABLE();
}
void
_PyDict_SendEvent(int watcher_bits,
PyDict_WatchEvent event,
@ -5744,9 +5763,18 @@ _PyDict_SendEvent(int watcher_bits,
if (watcher_bits & 1) {
PyDict_WatchCallback cb = interp->dict_state.watchers[i];
if (cb && (cb(event, (PyObject*)mp, key, value) < 0)) {
// some dict modification paths (e.g. PyDict_Clear) can't raise, so we
// can't propagate exceptions from dict watchers.
PyErr_WriteUnraisable((PyObject *)mp);
// We don't want to resurrect the dict by potentially having an
// unraisablehook keep a reference to it, so we don't pass the
// dict as context, just an informative string message. Dict
// repr can call arbitrary code, so we invent a simpler version.
PyObject *context = PyUnicode_FromFormat(
"%s watcher callback for <dict at %p>",
dict_event_name(event), mp);
if (context == NULL) {
context = Py_NewRef(Py_None);
}
PyErr_WriteUnraisable(context);
Py_DECREF(context);
}
}
watcher_bits >>= 1;

38
Objects/funcobject.c
View File

@ -8,6 +8,20 @@
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "structmember.h" // PyMemberDef
static PyObject* func_repr(PyFunctionObject *op);
static const char *
func_event_name(PyFunction_WatchEvent event) {
switch (event) {
#define CASE(op) \
case PyFunction_EVENT_##op: \
return "PyFunction_EVENT_" #op;
PY_FOREACH_FUNC_EVENT(CASE)
#undef CASE
}
Py_UNREACHABLE();
}
static void
notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event,
PyFunctionObject *func, PyObject *new_value)
@ -21,7 +35,21 @@ notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event,
// callback must be non-null if the watcher bit is set
assert(cb != NULL);
if (cb(event, func, new_value) < 0) {
PyErr_WriteUnraisable((PyObject *) func);
// Don't risk resurrecting the func if an unraisablehook keeps a
// reference; pass a string as context.
PyObject *context = NULL;
PyObject *repr = func_repr(func);
if (repr != NULL) {
context = PyUnicode_FromFormat(
"%s watcher callback for %U",
func_event_name(event), repr);
Py_DECREF(repr);
}
if (context == NULL) {
context = Py_NewRef(Py_None);
}
PyErr_WriteUnraisable(context);
Py_DECREF(context);
}
}
i++;
@ -33,6 +61,7 @@ static inline void
handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func,
PyObject *new_value)
{
assert(Py_REFCNT(func) > 0);
PyInterpreterState *interp = _PyInterpreterState_GET();
assert(interp->_initialized);
if (interp->active_func_watchers) {
@ -766,7 +795,14 @@ func_clear(PyFunctionObject *op)
static void
func_dealloc(PyFunctionObject *op)
{
assert(Py_REFCNT(op) == 0);
Py_SET_REFCNT(op, 1);
handle_func_event(PyFunction_EVENT_DESTROY, op, NULL);
if (Py_REFCNT(op) > 1) {
Py_SET_REFCNT(op, Py_REFCNT(op) - 1);
return;
}
Py_SET_REFCNT(op, 0);
_PyObject_GC_UNTRACK(op);
if (op->func_weakreflist != NULL) {
PyObject_ClearWeakRefs((PyObject *) op);