Skip to content

Commit

Permalink
pythongh-124872: Back up exception before calling PyContext_WatchCall…
Browse files Browse the repository at this point in the history
…back

I believe that the value of a simpler API (and defense against poorly
written callbacks) outweighs the cost of backing up and restoring the
thread's exception state.
  • Loading branch information
rhansen committed Oct 12, 2024
1 parent 330c527 commit c5789a9
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 6 deletions.
9 changes: 3 additions & 6 deletions Doc/c-api/contextvars.rst
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,13 @@ Context object management functions:
Context object watcher callback function. The object passed to the callback
is event-specific; see :c:type:`PyContextEvent` for details.
Any pending exception is cleared before the callback is called and restored
after the callback returns.
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_FormatUnraisable`. 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.14
Expand Down
3 changes: 3 additions & 0 deletions Include/cpython/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ typedef enum {
* Context object watcher callback function. The object passed to the callback
* is event-specific; see PyContextEvent for details.
*
* Any pending exception is cleared before the callback is called and restored
* after the callback returns.
*
* if the callback returns with an exception set, it must return -1. Otherwise
* it should return 0
*/
Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_capi/test_watchers.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,16 @@ def _in_context(stack):
ctx.run(_in_context, stack)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")

def test_exception_save(self):
with self.context_watcher(2):
with catch_unraisable_exception() as cm:
def _in_context():
raise RuntimeError("test")

with self.assertRaisesRegex(RuntimeError, "test"):
contextvars.copy_context().run(_in_context)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")

def test_clear_out_of_range_watcher_id(self):
with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"):
_testcapi.clear_context_watcher(-1)
Expand Down
2 changes: 2 additions & 0 deletions Python/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,13 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
if (bits & 1) {
PyContext_WatchCallback cb = interp->context_watchers[i];
assert(cb != NULL);
PyObject *exc = _PyErr_GetRaisedException(ts);
if (cb(event, ctx) < 0) {
PyErr_FormatUnraisable(
"Exception ignored in %s watcher callback for %R",
context_event_name(event), ctx);
}
_PyErr_SetRaisedException(ts, exc);
}
i++;
bits >>= 1;
Expand Down

0 comments on commit c5789a9

Please sign in to comment.