gh-102381: don't call watcher callback with dead object (#102382) · python/cpython@1e703a4 · GitHub
Skip to content

Commit 1e703a4

Browse files
carljmYhg1s
carljm
and
Yhg1s
authored
gh-102381: don't call watcher callback with dead object (#102382)
Co-authored-by: T. Wouters <thomas@python.org>
1 parent a33ca2a commit 1e703a4

12 files changed

Lines changed: 243 additions & 38 deletions

File tree

‎Doc/c-api/code.rst‎

Lines changed: 14 additions & 3 deletions

‎Doc/c-api/dict.rst‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,26 @@ Dictionary Objects
298298
dictionary.
299299
300300
The callback may inspect but must not modify *dict*; doing so could have
301-
unpredictable effects, including infinite recursion.
301+
unpredictable effects, including infinite recursion. Do not trigger Python
302+
code execution in the callback, as it could modify the dict as a side effect.
303+
304+
If *event* is ``PyDict_EVENT_DEALLOCATED``, taking a new reference in the
305+
callback to the about-to-be-destroyed dictionary will resurrect it and
306+
prevent it from being freed at this time. When the resurrected object is
307+
destroyed later, any watcher callbacks active at that time will be called
308+
again.
302309
303310
Callbacks occur before the notified modification to *dict* takes place, so
304311
the prior state of *dict* can be inspected.
305312
306-
If the callback returns with an exception set, it must return ``-1``; this
307-
exception will be printed as an unraisable exception using
308-
:c:func:`PyErr_WriteUnraisable`. Otherwise it should return ``0``.
313+
If the callback sets an exception, it must return ``-1``; this exception will
314+
be printed as an unraisable exception using :c:func:`PyErr_WriteUnraisable`.
315+
Otherwise it should return ``0``.
316+
317+
There may already be a pending exception set on entry to the callback. In
318+
this case, the callback should return ``0`` with the same exception still
319+
set. This means the callback may not call any other API that can set an
320+
exception unless it saves and clears the exception state first, and restores
321+
it before returning.
309322
310323
.. versionadded:: 3.12

‎Doc/c-api/function.rst‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,19 @@ There are a few functions specific to Python functions.
173173
runtime behavior depending on optimization decisions, it does not change
174174
the semantics of the Python code being executed.
175175
176-
If the callback returns with an exception set, it must return ``-1``; this
177-
exception will be printed as an unraisable exception using
178-
:c:func:`PyErr_WriteUnraisable`. Otherwise it should return ``0``.
176+
If *event* is ``PyFunction_EVENT_DESTROY``, Taking a reference in the
177+
callback to the about-to-be-destroyed function will resurrect it, preventing
178+
it from being freed at this time. When the resurrected object is destroyed
179+
later, any watcher callbacks active at that time will be called again.
180+
181+
If the callback sets an exception, it must return ``-1``; this exception will
182+
be printed as an unraisable exception using :c:func:`PyErr_WriteUnraisable`.
183+
Otherwise it should return ``0``.
184+
185+
There may already be a pending exception set on entry to the callback. In
186+
this case, the callback should return ``0`` with the same exception still
187+
set. This means the callback may not call any other API that can set an
188+
exception unless it saves and clears the exception state first, and restores
189+
it before returning.
179190
180191
.. versionadded:: 3.12

‎Include/cpython/code.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,14 @@ PyAPI_FUNC(int) PyCode_Addr2Line(PyCodeObject *, int);
224224

225225
PyAPI_FUNC(int) PyCode_Addr2Location(PyCodeObject *, int, int *, int *, int *, int *);
226226

227-
typedef enum PyCodeEvent {
228-
PY_CODE_EVENT_CREATE,
229-
PY_CODE_EVENT_DESTROY
227+
#define PY_FOREACH_CODE_EVENT(V) \
228+
V(CREATE) \
229+
V(DESTROY)
230+
231+
typedef enum {
232+
#define PY_DEF_EVENT(op) PY_CODE_EVENT_##op,
233+
PY_FOREACH_CODE_EVENT(PY_DEF_EVENT)
234+
#undef PY_DEF_EVENT
230235
} PyCodeEvent;
231236

232237

@@ -236,7 +241,7 @@ typedef enum PyCodeEvent {
236241
* The callback is invoked with a borrowed reference to co, after it is
237242
* created and before it is destroyed.
238243
*
239-
* If the callback returns with an exception set, it must return -1. Otherwise
244+
* If the callback sets an exception, it must return -1. Otherwise
240245
* it should return 0.
241246
*/
242247
typedef int (*PyCode_WatchCallback)(

‎Include/cpython/dictobject.h‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ typedef struct {
1616

1717
/* Dictionary version: globally unique, value change each time
1818
the dictionary is modified */
19-
#ifdef Py_BUILD_CORE
19+
#ifdef Py_BUILD_CORE
2020
uint64_t ma_version_tag;
2121
#else
2222
Py_DEPRECATED(3.12) uint64_t ma_version_tag;
23-
#endif
23+
#endif
2424

2525
PyDictKeysObject *ma_keys;
2626

@@ -90,13 +90,18 @@ PyAPI_FUNC(PyObject *) _PyDictView_Intersect(PyObject* self, PyObject *other);
9090

9191
/* Dictionary watchers */
9292

93+
#define PY_FOREACH_DICT_EVENT(V) \
94+
V(ADDED) \
95+
V(MODIFIED) \
96+
V(DELETED) \
97+
V(CLONED) \
98+
V(CLEARED) \
99+
V(DEALLOCATED)
100+
93101
typedef enum {
94-
PyDict_EVENT_ADDED,
95-
PyDict_EVENT_MODIFIED,
96-
PyDict_EVENT_DELETED,
97-
PyDict_EVENT_CLONED,
98-
PyDict_EVENT_CLEARED,
99-
PyDict_EVENT_DEALLOCATED,
102+
#define PY_DEF_EVENT(EVENT) PyDict_EVENT_##EVENT,
103+
PY_FOREACH_DICT_EVENT(PY_DEF_EVENT)
104+
#undef PY_DEF_EVENT
100105
} PyDict_WatchEvent;
101106

102107
// Callback to be invoked when a watched dict is cleared, dealloced, or modified.

‎Include/cpython/funcobject.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,17 +131,17 @@ PyAPI_DATA(PyTypeObject) PyStaticMethod_Type;
131131
PyAPI_FUNC(PyObject *) PyClassMethod_New(PyObject *);
132132
PyAPI_FUNC(PyObject *) PyStaticMethod_New(PyObject *);
133133

134-
#define FOREACH_FUNC_EVENT(V) \
135-
V(CREATE) \
136-
V(DESTROY) \
137-
V(MODIFY_CODE) \
138-
V(MODIFY_DEFAULTS) \
134+
#define PY_FOREACH_FUNC_EVENT(V) \
135+
V(CREATE) \
136+
V(DESTROY) \
137+
V(MODIFY_CODE) \
138+
V(MODIFY_DEFAULTS) \
139139
V(MODIFY_KWDEFAULTS)
140140

141141
typedef enum {
142-
#define DEF_EVENT(EVENT) PyFunction_EVENT_##EVENT,
143-
FOREACH_FUNC_EVENT(DEF_EVENT)
144-
#undef DEF_EVENT
142+
#define PY_DEF_EVENT(EVENT) PyFunction_EVENT_##EVENT,
143+
PY_FOREACH_FUNC_EVENT(PY_DEF_EVENT)
144+
#undef PY_DEF_EVENT
145145
} PyFunction_WatchEvent;
146146

147147
/*

‎Include/internal/pycore_dict.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ _PyDict_NotifyEvent(PyDict_WatchEvent event,
164164
PyObject *key,
165165
PyObject *value)
166166
{
167+
assert(Py_REFCNT((PyObject*)mp) > 0);
167168
int watcher_bits = mp->ma_version_tag & DICT_VERSION_MASK;
168169
if (watcher_bits) {
169170
_PyDict_SendEvent(watcher_bits, event, mp, key, value);

‎Lib/test/test_capi/test_watchers.py‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,21 @@ def test_error(self):
109109
self.watch(wid, d)
110110
with catch_unraisable_exception() as cm:
111111
d["foo"] = "bar"
112-
self.assertIs(cm.unraisable.object, d)
112+
self.assertIn(
113+
"PyDict_EVENT_ADDED watcher callback for <dict at",
114+
cm.unraisable.object
115+
)
113116
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
114117
self.assert_events([])
115118

119+
def test_dealloc_error(self):
120+
d = {}
121+
with self.watcher(kind=self.ERROR) as wid:
122+
self.watch(wid, d)
123+
with catch_unraisable_exception() as cm:
124+
del d
125+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
126+
116127
def test_two_watchers(self):
117128
d1 = {}
118129
d2 = {}
@@ -389,6 +400,25 @@ def test_code_object_events_dispatched(self):
389400
del co4
390401
self.assert_event_counts(0, 0, 0, 0)
391402

403+
def test_error(self):
404+
with self.code_watcher(2):
405+
with catch_unraisable_exception() as cm:
406+
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
407+
408+
self.assertEqual(
409+
cm.unraisable.object,
410+
f"PY_CODE_EVENT_CREATE watcher callback for {co!r}"
411+
)
412+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
413+
414+
def test_dealloc_error(self):
415+
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
416+
with self.code_watcher(2):
417+
with catch_unraisable_exception() as cm:
418+
del co
419+
420+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
421+
392422
def test_clear_out_of_range_watcher_id(self):
393423
with self.assertRaisesRegex(ValueError, r"Invalid code watcher ID -1"):
394424
_testcapi.clear_code_watcher(-1)
@@ -479,7 +509,25 @@ def watcher(*args):
479509
def myfunc():
480510
pass
481511

482-
self.assertIs(cm.unraisable.object, myfunc)
512+
self.assertEqual(
513+
cm.unraisable.object,
514+
f"PyFunction_EVENT_CREATE watcher callback for {myfunc!r}"
515+
)
516+
517+
def test_dealloc_watcher_raises_error(self):
518+
class MyError(Exception):
519+
pass
520+
521+
def watcher(*args):
522+
raise MyError("testing 123")
523+
524+
def myfunc():
525+
pass
526+
527+
with self.add_watcher(watcher):
528+
with catch_unraisable_exception() as cm:
529+
del myfunc
530+
483531
self.assertIsInstance(cm.unraisable.exc_value, MyError)
484532

485533
def test_clear_out_of_range_watcher_id(self):

‎Modules/_testcapi/watchers.c‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,13 @@ noop_code_event_handler(PyCodeEvent event, PyCodeObject *co)
317317
return 0;
318318
}
319319

320+
static int
321+
error_code_event_handler(PyCodeEvent event, PyCodeObject *co)
322+
{
323+
PyErr_SetString(PyExc_RuntimeError, "boom!");
324+
return -1;
325+
}
326+
320327
static PyObject *
321328
add_code_watcher(PyObject *self, PyObject *which_watcher)
322329
{
@@ -333,7 +340,11 @@ add_code_watcher(PyObject *self, PyObject *which_watcher)
333340
num_code_object_created_events[1] = 0;
334341
num_code_object_destroyed_events[1] = 0;
335342
}
343+
else if (which_l == 2) {
344+
watcher_id = PyCode_AddWatcher(error_code_event_handler);
345+
}
336346
else {
347+
PyErr_Format(PyExc_ValueError, "invalid watcher %d", which_l);
337348
return NULL;
338349
}
339350
if (watcher_id < 0) {
@@ -672,7 +683,7 @@ _PyTestCapi_Init_Watchers(PyObject *mod)
672683
PyFunction_EVENT_##event)) { \
673684
return -1; \
674685
}
675-
FOREACH_FUNC_EVENT(ADD_EVENT);
686+
PY_FOREACH_FUNC_EVENT(ADD_EVENT);
676687
#undef ADD_EVENT
677688

678689
return 0;

‎Objects/codeobject.c‎

Lines changed: 37 additions & 1 deletion

0 commit comments

Comments
 (0)