Skip to content

Commit 6d0b789

Browse files
carljmcarljm
committed
gh-102381: don't call watcher callback with dead object
1 parent 71db5db commit 6d0b789

File tree

6 files changed

+107
-7
lines changed

6 files changed

+107
-7
lines changed

‎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: 47 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.assertEqual(
113+
cm.unraisable.object,
114+
f"watcher callback for <dict at {hex(id(d))}>"
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,22 @@ 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(cm.unraisable.object, f"watcher callback for {co!r}")
409+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
410+
411+
def test_dealloc_error(self):
412+
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
413+
with self.code_watcher(2):
414+
with catch_unraisable_exception() as cm:
415+
del co
416+
417+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
418+
392419
def test_clear_out_of_range_watcher_id(self):
393420
with self.assertRaisesRegex(ValueError, r"Invalid code watcher ID -1"):
394421
_testcapi.clear_code_watcher(-1)
@@ -479,7 +506,25 @@ def watcher(*args):
479506
def myfunc():
480507
pass
481508

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

485530
def test_clear_out_of_range_watcher_id(self):

‎Modules/_testcapi/watchers.c

Lines changed: 11 additions & 0 deletions
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) {

‎Objects/codeobject.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111
#include "pycore_tuple.h" // _PyTuple_ITEMS()
1212
#include "clinic/codeobject.c.h"
1313

14+
static PyObject* code_repr(PyCodeObject *co);
15+
1416
static void
1517
notify_code_watchers(PyCodeEvent event, PyCodeObject *co)
1618
{
19+
assert(Py_REFCNT(co) > 0);
1720
PyInterpreterState *interp = _PyInterpreterState_GET();
1821
assert(interp->_initialized);
1922
uint8_t bits = interp->active_code_watchers;
@@ -25,7 +28,13 @@ notify_code_watchers(PyCodeEvent event, PyCodeObject *co)
2528
// callback must be non-null if the watcher bit is set
2629
assert(cb != NULL);
2730
if (cb(event, co) < 0) {
28-
PyErr_WriteUnraisable((PyObject *) co);
31+
// Don't risk resurrecting the object if an unraisablehook keeps
32+
// a reference; pass a string as context.
33+
PyObject *repr = code_repr(co);
34+
PyObject *context = PyUnicode_FromFormat("watcher callback for %U", repr);
35+
PyErr_WriteUnraisable(context);
36+
Py_DECREF(context);
37+
Py_DECREF(repr);
2938
}
3039
}
3140
i++;
@@ -1667,7 +1676,14 @@ code_new_impl(PyTypeObject *type, int argcount, int posonlyargcount,
16671676
static void
16681677
code_dealloc(PyCodeObject *co)
16691678
{
1679+
assert(Py_REFCNT(co) == 0);
1680+
Py_SET_REFCNT(co, 1);
16701681
notify_code_watchers(PY_CODE_EVENT_DESTROY, co);
1682+
if (Py_REFCNT(co) > 1) {
1683+
Py_DECREF(co);
1684+
return;
1685+
}
1686+
Py_SET_REFCNT(co, 0);
16711687

16721688
if (co->co_extra != NULL) {
16731689
PyInterpreterState *interp = _PyInterpreterState_GET();

‎Objects/dictobject.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,7 +2308,14 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
23082308
static void
23092309
dict_dealloc(PyDictObject *mp)
23102310
{
2311+
assert(Py_REFCNT(mp) == 0);
2312+
Py_SET_REFCNT(mp, 1);
23112313
_PyDict_NotifyEvent(PyDict_EVENT_DEALLOCATED, mp, NULL, NULL);
2314+
if(Py_REFCNT(mp) > 1) {
2315+
Py_DECREF(mp);
2316+
return;
2317+
}
2318+
Py_SET_REFCNT(mp, 0);
23122319
PyDictValues *values = mp->ma_values;
23132320
PyDictKeysObject *keys = mp->ma_keys;
23142321
Py_ssize_t i, n;
@@ -5744,9 +5751,13 @@ _PyDict_SendEvent(int watcher_bits,
57445751
if (watcher_bits & 1) {
57455752
PyDict_WatchCallback cb = interp->dict_state.watchers[i];
57465753
if (cb && (cb(event, (PyObject*)mp, key, value) < 0)) {
5747-
// some dict modification paths (e.g. PyDict_Clear) can't raise, so we
5748-
// can't propagate exceptions from dict watchers.
5749-
PyErr_WriteUnraisable((PyObject *)mp);
5754+
// We don't want to resurrect the dict by potentially having an
5755+
// unraisablehook keep a reference to it, so we don't pass the
5756+
// dict as context, just an informative string message. Dict
5757+
// repr can call arbitrary code, so we invent a simpler version.
5758+
PyObject *context = PyUnicode_FromFormat("watcher callback for <dict at %p>", mp);
5759+
PyErr_WriteUnraisable(context);
5760+
Py_DECREF(context);
57505761
}
57515762
}
57525763
watcher_bits >>= 1;

‎Objects/funcobject.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include "pycore_pyerrors.h" // _PyErr_Occurred()
99
#include "structmember.h" // PyMemberDef
1010

11+
static PyObject* func_repr(PyFunctionObject *op);
12+
1113
static void
1214
notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event,
1315
PyFunctionObject *func, PyObject *new_value)
@@ -21,7 +23,13 @@ notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event,
2123
// callback must be non-null if the watcher bit is set
2224
assert(cb != NULL);
2325
if (cb(event, func, new_value) < 0) {
24-
PyErr_WriteUnraisable((PyObject *) func);
26+
// Don't risk resurrecting the func if an unraisablehook keeps a
27+
// reference; pass a string as context.
28+
PyObject *repr = func_repr(func);
29+
PyObject *context = PyUnicode_FromFormat("watcher callback for %U", repr);
30+
PyErr_WriteUnraisable(context);
31+
Py_DECREF(context);
32+
Py_DECREF(repr);
2533
}
2634
}
2735
i++;
@@ -33,6 +41,7 @@ static inline void
3341
handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func,
3442
PyObject *new_value)
3543
{
44+
assert(Py_REFCNT(func) > 0);
3645
PyInterpreterState *interp = _PyInterpreterState_GET();
3746
assert(interp->_initialized);
3847
if (interp->active_func_watchers) {
@@ -766,7 +775,14 @@ func_clear(PyFunctionObject *op)
766775
static void
767776
func_dealloc(PyFunctionObject *op)
768777
{
778+
assert(Py_REFCNT(op) == 0);
779+
Py_SET_REFCNT(op, 1);
769780
handle_func_event(PyFunction_EVENT_DESTROY, op, NULL);
781+
if (Py_REFCNT(op) > 1) {
782+
Py_DECREF(op);
783+
return;
784+
}
785+
Py_SET_REFCNT(op, 0);
770786
_PyObject_GC_UNTRACK(op);
771787
if (op->func_weakreflist != NULL) {
772788
PyObject_ClearWeakRefs((PyObject *) op);

0 commit comments

Comments
 (0)