8000 gh-124872: Mark the thread's default context as entered · python/cpython@5337fc3 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 5337fc3

Browse files
rhansenrhansen
committed
gh-124872: Mark the thread's default context as entered
Starting with commit 843d28f (temporarily reverted in d3c82b9 and restored in commit bee112a), it is now technically possible to access a thread's default context created by `context_get`. Mark that context as entered so that users cannot push that context onto the thread's stack a second time, which would cause a cycle. This change also causes a `CONTEXT_SWITCHED` event to be emitted when the default context is created, which might be important in some use cases. Also exit the default context when the thread exits, for symmetry and in case the user wants to re-enter it for some reason. (Even if the `CONTEXT_SWITCHED` event is removed, entering the default context is good defensive practice, and the consistent treatment of all contexts on the stack makes it easier to understand the code.)
1 parent f203d1c commit 5337fc3

File tree

7 files changed

+155
-13
lines changed

7 files changed

+155
-13
lines changed

‎Doc/c-api/contextvars.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ Context object management functions:
126126
- ``Py_CONTEXT_SWITCHED``: The :term:`current context` has switched to a
127127
different context. The object passed to the watch callback is the
128128
now-current :class:`contextvars.Context` object, or None if no context is
129-
current.
129+
current. The thread executing the callback is guaranteed to be the thread
130+
that experienced the context switch.
130131
131132
8000 .. versionadded:: 3.14
132133

‎Include/cpython/context.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ typedef enum {
3131
/*
3232
* The current context has switched to a different context. The object
3333
* passed to the watch callback is the now-current contextvars.Context
34-
* object, or None if no context is current.
34+
* object, or None if no context is current. The thread executing the
35+
* callback is guaranteed to be the thread that experienced the context
36+
* switch.
3537
*/
3638
Py_CONTEXT_SWITCHED = 1,
3739
} PyContextEvent;

‎Include/internal/pycore_context.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ extern PyTypeObject _PyContextTokenMissing_Type;
1515

1616
PyStatus _PyContext_Init(PyInterpreterState *);
1717

18+
// Exits any thread-owned contexts (see context_get) at the top of the thread's
19+
// context stack. Logs a warning via PyErr_FormatUnraisable if the thread's
20+
// context stack is non-empty afterwards (those contexts can never be exited or
21+
// re-entered).
22+
void _PyContext_ExitThreadOwned(PyThreadState *);
23+
1824

1925
/* other API */
2026

@@ -27,7 +33,11 @@ struct _pycontextobject {
2733
PyContext *ctx_prev;
2834
PyHamtObject *ctx_vars;
2935
PyObject *ctx_weakreflist;
30-
int ctx_entered;
36+
_Bool ctx_entered:1;
37+
// True for the thread's default context created by context_get. Used to
38+
// safely determine whether the base context can be exited when clearing a
39+
// PyThreadState.
40+
_Bool ctx_owned_by_thread:1;
3141
};
3242

3343

‎Lib/test/test_capi/test_watchers.py

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
import threading
12
import unittest
23
import contextvars
34

45
from contextlib import contextmanager, ExitStack
56
from test.support import (
67
catch_unraisable_exception, import_helper,
7-
gc_collect, suppress_immortalization)
8+
gc_collect, suppress_immortalization, threading_helper)
89

910

1011
# Skip this test if the _testcapi module isn't available.
@@ -659,5 +660,75 @@ def test_exit_base_context(self):
659660
ctx.run(lambda: None)
660661
self.assertEqual(switches, [ctx, None])
661662

663+
def test_reenter_default_context(self):
664+
_testcapi.clear_context_stack()
665+
# contextvars.copy_context() creates the thread's default context (via
666+
# the context_get C function).
667+
ctx = contextvars.copy_context()
668+
with self.context_watcher(0) as switches:
669+
ctx.run(lambda: None)
670+
self.assertEqual(len(switches), 2)
671+
self.assertEqual(switches[0], ctx)
672+
base_ctx = switches[1]
673+
self.assertIsNotNone(base_ctx)
674+
self.assertIsNot(base_ctx, ctx)
675+
with self.assertRaisesRegex(RuntimeError, 'already entered'):
676+
base_ctx.run(lambda: None)
677+
678+
def test_default_context_enter(self):
679+
_testcapi.clear_context_stack()
680+
with self.context_watcher(0) as switches:
681+
ctx = contextvars.copy_context()
682+
ctx.run(lambda: None)
683+
self.assertEqual(len(switches), 3)
684+
base_ctx = switches[0]
685+
self.assertIsNotNone(base_ctx)
686+
self.assertEqual(switches, [base_ctx, ctx, base_ctx])
687+
688+
@threading_helper.requires_working_threading()
689+
def test_default_context_exit_during_thread_cleanup(self):
690+
# Context watchers are per-interpreter, not per-thread.
691+
with self.context_watcher(0) as switches:
692+
def _thread_main():
693+
_testcapi.clear_context_stack()
694+
# contextvars.copy_context() creates the thread's default
695+
# context (via the context_get C function).
696+
contextvars.copy_context()
697+
# This test only cares about the final switch that happens when
698+
# exiting the thread's default context during thread cleanup.
699+
switches.clear()
700+
701+
thread = threading.Thread(target=_thread_main)
702+
thread.start()
703+
threading_helper.join_thread(thread)
704+
self.assertEqual(switches, [None])
705+
706+
@threading_helper.requires_working_threading()
707+
def test_thread_cleanup_with_entered_context(self):
708+
unraisables = []
709+
try:
710+
with catch_unraisable_exception() as cm:
711+
with self.context_watcher(0) as switches:
712+
def _thread_main():
713+
_testcapi.clear_context_stack()
714+
ctx = contextvars.copy_context()
715+
_testcapi.context_enter(ctx)
716+
switches.clear()
717+
718+
thread = threading.Thread(target=_thread_main)
719+
thread.start()
720+
threading_helper.join_thread(thread)
721+
unraisables.append(cm.unraisable)
722+
self.assertEqual(switches, [])
723+
self.assertEqual(len(unraisables), 1)
724+
self.assertIsNotNone(unraisables[0])
725+
self.assertRegex(unraisables[0].err_msg,
726+
r'^Exception ignored during reset of thread state')
727+
self.assertRegex(str(unraisables[0].exc_value), r'still entered')
728+
finally:
729+
# Break reference cycle
730+
unraisables = None
731+
732+
662733
if __name__ == "__main__":
663734
unittest.main()

‎Modules/_testcapi/watchers.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,15 @@ clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
724724
Py_RETURN_NONE;
725725
}
726726

727+
static PyObject *
728+
context_enter(PyObject *self, PyObject *ctx)
729+
{
730+
if (PyContext_Enter(ctx)) {
731+
return NULL;
732+
}
733+
Py_RETURN_NONE;
734+
}
735+
727736
static PyObject *
728737
get_context_switches(PyObject *Py_UNUSED(self), PyObject *watcher_id)
729738
{
@@ -841,6 +850,7 @@ static PyMethodDef test_methods[] = {
841850
{"add_context_watcher", add_context_watcher, METH_O, NULL},
842851
{"clear_context_watcher", clear_context_watcher, METH_O, NULL},
843852
{"clear_context_stack", clear_context_stack, METH_NOARGS, NULL},
853+
{"context_enter", context_enter, METH_O, NULL},
844854
{"get_context_switches", get_context_switches, METH_O, NULL},
845855
{"allocate_too_many_context_watchers",
846856
(PyCFunction) allocate_too_many_context_watchers, METH_NOARGS, NULL},

‎Python/context.c

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ context_event_name(PyContextEvent event) {
113113
static void
114114
notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
115115
{
116+
// The callbacks are registered on the interpreter, not on the thread, so
117+
// the only way callbacks can know which thread changed is by calling the
118+
// callbacks from the affected thread.
119+
assert(ts == _PyThreadState_GET());
116120
if (ctx == NULL) {
117121
// This will happen after exiting the last context in the stack, which
118122
// can occur if context_get was never called before entering a context
@@ -184,8 +188,9 @@ static inline void
184188
context_switched(PyThreadState *ts)
185189
{
186190
ts->context_ver++;
187-
// ts->context is used instead of context_get() because context_get() might
188-
// throw if ts->context is NULL.
191+
// ts->context is used instead of context_get() because if ts->context is
192+
// NULL, context_get() will either call context_switched -- causing a
193+
// double notification -- or throw.
189194
notify_context_watchers(ts, Py_CONTEXT_SWITCHED, ts->context);
190195
}
191196

@@ -244,6 +249,7 @@ _PyContext_Exit(PyThreadState *ts, PyObject *octx)
244249

245250
ctx->ctx_prev = NULL;
246251
ctx->ctx_entered = 0;
252+
ctx->ctx_owned_by_thread = 0;
247253
context_switched(ts);
248254
return 0;
249255
}
@@ -257,6 +263,37 @@ PyContext_Exit(PyObject *octx)
257263
}
258264

259265

266+
void
267+
_PyContext_ExitThreadOwned(PyThreadState *ts)
268+
{
269+
assert(ts != NULL);
270+
// notify_context_watchers requires the notification to come from the
271+
// affected thread, so we can only exit the context(s) if ts belongs to the
272+
// current thread.
273+
_Bool on_thread = ts == _PyThreadState_GET();
274+
while (ts->context != NULL
275+
&& PyContext_CheckExact(ts->context)
276+
&& ((PyContext *)ts->context)->ctx_owned_by_thread
277+
&& on_thread) {
278+
if (_PyContext_Exit(ts, ts->context)) {
279+
Py_UNREACHABLE();
280+
}
281+
}
282+
if (ts->context != NULL) {
283+
// This intentionally does not use tstate variants of these functions
284+
// (e.g., _PyErr_GetRaisedException(ts)) because ts might not belong to
285+
// the current thread.
286+
PyObject *exc = PyErr_GetRaisedException();
287+
PyErr_SetString(PyExc_RuntimeError,
288+
"contextvars.Context object(s) still entered during "
289+
"thread state reset");
290+
PyErr_FormatUnraisable(
291+
"Exception ignored during reset of thread state %p", ts);
292+
PyErr_SetRaisedException(exc);
293+
}
294+
}
295+
296+
260297
PyObject *
261298
PyContextVar_New(const char *name, PyObject *def)
262299
{
@@ -433,6 +470,7 @@ _context_alloc(void)
433470
ctx->ctx_vars = NULL;
434471
ctx->ctx_prev = NULL;
435472
ctx->ctx_entered = 0;
473+
ctx->ctx_owned_by_thread = 0;
436474
ctx->ctx_weakreflist = NULL;
437475

438476
return ctx;
@@ -478,15 +516,21 @@ context_get(void)
478516
{
479517
PyThreadState *ts = _PyThreadState_GET();
480518
assert(ts != NULL);
481-
PyContext *current_ctx = (PyContext *)ts->context;
482-
if (current_ctx == NULL) {
483-
current_ctx = context_new_empty();
484-
if (current_ctx == NULL) {
485-
return NULL;
519+
if (ts->context == NULL) {
520+
PyContext *ctx = context_new_empty();
521+
if (ctx != NULL) {
522+
if (_PyContext_Enter(ts, (PyObject *)ctx)) {
523+
Py_UNREACHABLE();
524+
}
525+
ctx->ctx_owned_by_thread = 1;
486526
}
487-
ts->context = (PyObject *)current_ctx;
527+
assert(ts->context == (PyObject *)ctx);
528+
Py_CLEAR(ctx); // _PyContext_Enter created its own ref.
488529
}
489-
return current_ctx;
530+
// The current context may be NULL if the above context_new_empty() call
531+
// failed.
532+
assert(ts->context == NULL || PyContext_CheckExact(ts->context));
533+
return (PyContext *)ts->context;
490534
}
491535

492536
static int

‎Python/pystate.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,6 +1650,10 @@ PyThreadState_Clear(PyThreadState *tstate)
16501650
"PyThreadState_Clear: warning: thread still has a frame\n");
16511651
}
16521652

1653+
// This calls callbacks registered with PyContext_AddWatcher and can call
1654+
// sys.unraisablehook.
1655+
_PyContext_ExitThreadOwned(tstate);
1656+
16531657
/* At this point tstate shouldn't be used any more,
16541658
neither to run Python code nor for other uses.
16551659

0 commit comments

Comments
 (0)
0