Skip to content

Commit acf0fdb

Browse files
committed
gh-124872: Back up exception before calling PyContext_WatchCallback
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.
1 parent 120729d commit acf0fdb

File tree

4 files changed

+18
-6
lines changed

4 files changed

+18
-6
lines changed

Doc/c-api/contextvars.rst

+3-6
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,13 @@ Context object management functions:
135135
Otherwise, the callback is invoked before the deactivation of *ctx* as the current context
136136
and the restoration of the previous contex object for the current thread.
137137
138+
Any pending exception is cleared before the callback is called and restored
139+
after the callback returns.
140+
138141
If the callback returns with an exception set, it must return ``-1``; this
139142
exception will be printed as an unraisable exception using
140143
:c:func:`PyErr_FormatUnraisable`. Otherwise it should return ``0``.
141144
142-
There may already be a pending exception set on entry to the callback. In
143-
this case, the callback should return ``0`` with the same exception still
144-
set. This means the callback may not call any other API that can set an
145-
exception unless it saves and clears the exception state first, and restores
146-
it before returning.
147-
148145
.. versionadded:: 3.14
149146
150147

Include/cpython/context.h

+3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ typedef enum {
3838
* The callback is invoked with the event and a reference to
3939
* the context after its entered and before its exited.
4040
*
41+
* Any pending exception is cleared before the callback is called and restored
42+
* after the callback returns.
43+
*
4144
* if the callback returns with an exception set, it must return -1. Otherwise
4245
* it should return 0
4346
*/

Lib/test/test_capi/test_watchers.py

+10
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,16 @@ def _in_context(stack):
640640
ctx.run(_in_context, stack)
641641
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
642642

643+
def test_exception_save(self):
644+
with self.context_watcher(2):
645+
with catch_unraisable_exception() as cm:
646+
def _in_context():
647+
raise RuntimeError("test")
648+
649+
with self.assertRaisesRegex(RuntimeError, "test"):
650+
contextvars.copy_context().run(_in_context)
651+
self.assertEqual(str(cm.unraisable.exc_value), "boom!")
652+
643653
def test_clear_out_of_range_watcher_id(self):
644654
with self.assertRaisesRegex(ValueError, r"Invalid context watcher ID -1"):
645655
_testcapi.clear_context_watcher(-1)

Python/context.c

+2
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,13 @@ static void notify_context_watchers(PyContextEvent event, PyContext *ctx, PyThre
124124
if (bits & 1) {
125125
PyContext_WatchCallback cb = interp->context_watchers[i];
126126
assert(cb != NULL);
127+
PyObject *exc = _PyErr_GetRaisedException(ts);
127128
if (cb(event, ctx) < 0) {
128129
PyErr_FormatUnraisable(
129130
"Exception ignored in %s watcher callback for %R",
130131
context_event_name(event), ctx);
131132
}
133+
_PyErr_SetRaisedException(ts, exc);
132134
}
133135
i++;
134136
bits >>= 1;

0 commit comments

Comments
 (0)