Skip to content

Commit 6d59083

Browse files
serhiy-storchakaclaudepicnixz
authored
gh-116946: Implement the GC protocol for _tkinter tkapp and tktimertoken (GH-152310)
The _tkinter.tkapp and _tkinter.tktimertoken types never implemented the garbage collector protocol, so reference cycles through an interpreter's trace function or a timer handler's callback could not be collected. A pending timer is kept alive by the Tcl event loop, which fires it even after the Python token is dropped, so it is treated as a GC root (only its callback is traversed) and collecting it never cancels a live timer. The GC slots use a plain cast rather than the type-checking macro, since the collector may visit a surviving object at shutdown after module clearing has reset the global type pointers. Deallocation cancels any pending timer so its callback cannot run on freed memory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent c59c95f commit 6d59083

3 files changed

Lines changed: 101 additions & 13 deletions

File tree

Lib/test/test_tkinter/test_misc.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import collections.abc
22
import functools
3+
import gc
34
import platform
45
import sys
56
import textwrap
7+
import time
68
import unittest
79
import weakref
810
import tkinter
@@ -414,6 +416,36 @@ def test_createcommand_no_leak(self):
414416
support.gc_collect()
415417
self.assertIsNone(ref())
416418

419+
def test_gc_protocol(self):
420+
# gh-116946: _tkinter objects implement the GC protocol.
421+
self.assertTrue(gc.is_tracked(self.root))
422+
tok = self.root.tk.createtimerhandler(10_000_000, lambda: None)
423+
try:
424+
self.assertTrue(gc.is_tracked(tok))
425+
finally:
426+
tok.deletetimerhandler()
427+
428+
def test_timer_fires_after_gc(self):
429+
# gh-116946: a pending timer is kept alive by the Tcl event loop, not by
430+
# the garbage collector, so collecting it must not cancel it -- it must
431+
# still fire even when the Python token has been dropped.
432+
fired = []
433+
self.root.tk.createtimerhandler(1, lambda: fired.append(1))
434+
support.gc_collect()
435+
deadline = time.monotonic() + support.SHORT_TIMEOUT
436+
while not fired and time.monotonic() < deadline:
437+
self.root.update()
438+
self.assertEqual(fired, [1])
439+
440+
def test_pending_timer_at_shutdown(self):
441+
# gh-116946: the final garbage collection at interpreter shutdown must
442+
# not crash when it visits a timer that is still pending (its type has
443+
# already been cleared by the module's tp_clear).
444+
assert_python_ok('-c',
445+
'import tkinter\n'
446+
'interp = tkinter.Tcl()\n'
447+
'interp.tk.createtimerhandler(10_000_000, lambda: None)\n')
448+
417449
def test_option(self):
418450
self.addCleanup(self.root.option_clear)
419451
self.root.option_add('*Button.background', 'red')
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The internal :mod:`!_tkinter` ``tkapp`` and ``tktimertoken`` types now
2+
implement the garbage collector protocol, so reference cycles involving a
3+
Tcl interpreter or a timer handler can be collected.

Modules/_tkinter.c

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,8 @@ Tkapp_New(const char *screenName, const char *className,
635635
TkappObject *v;
636636
char *argv0;
637637

638-
v = PyObject_New(TkappObject, (PyTypeObject *) Tkapp_Type);
638+
PyTypeObject *tp = (PyTypeObject *)Tkapp_Type;
639+
v = (TkappObject *)tp->tp_alloc(tp, 0);
639640
if (v == NULL)
640641
return NULL;
641642

@@ -2943,7 +2944,8 @@ Tktt_New(PyObject *func)
29432944
{
29442945
TkttObject *v;
29452946

2946-
v = PyObject_New(TkttObject, (PyTypeObject *) Tktt_Type);
2947+
PyTypeObject *tp = (PyTypeObject *)Tktt_Type;
2948+
v = (TkttObject *)tp->tp_alloc(tp, 0);
29472949
if (v == NULL)
29482950
return NULL;
29492951

@@ -2954,16 +2956,41 @@ Tktt_New(PyObject *func)
29542956
return (TkttObject*)Py_NewRef(v);
29552957
}
29562958

2957-
static void
2958-
Tktt_Dealloc(PyObject *self)
2959+
/* Plain cast, not TkttObject_CAST: the GC can run at shutdown after
2960+
module_clear() has cleared the global Tktt_Type the macro checks against. */
2961+
2962+
static int
2963+
Tktt_Clear(PyObject *op)
29592964
{
2960-
TkttObject *v = TkttObject_CAST(self);
2961-
PyObject *func = v->func;
2962-
PyObject *tp = (PyObject *) Py_TYPE(self);
2965+
TkttObject *self = (TkttObject *)op;
2966+
Py_CLEAR(self->func);
2967+
return 0;
2968+
}
29632969

2964-
Py_XDECREF(func);
2970+
static int
2971+
Tktt_Traverse(PyObject *op, visitproc visit, void *arg)
2972+
{
2973+
TkttObject *self = (TkttObject *)op;
2974+
Py_VISIT(Py_TYPE(op));
2975+
/* Not the extra reference of a pending timer (see Tktt_New): it is owned
2976+
by the Tcl event loop, so the timer is a GC root, not part of a cycle. */
2977+
Py_VISIT(self->func);
2978+
return 0;
2979+
}
29652980

2966-
PyObject_Free(self);
2981+
static void
2982+
Tktt_Dealloc(PyObject *op)
2983+
{
2984+
TkttObject *self = (TkttObject *)op; /* see GC slots above */
2985+
PyTypeObject *tp = Py_TYPE(op);
2986+
PyObject_GC_UnTrack(op);
2987+
/* Cancel any pending timer so its callback cannot fire on freed memory. */
2988+
if (self->token != NULL) {
2989+
Tcl_DeleteTimerHandler(self->token);
2990+
self->token = NULL;
2991+
}
2992+
(void)Tktt_Clear(op);
2993+
tp->tp_free(op);
29672994
Py_DECREF(tp);
29682995
}
29692996

@@ -3257,11 +3284,31 @@ _tkinter_tkapp_willdispatch_impl(TkappObject *self)
32573284

32583285
/**** Tkapp Type Methods ****/
32593286

3287+
/* Plain casts -- see the Tktt GC slots above. */
3288+
3289+
static int
3290+
Tkapp_Clear(PyObject *op)
3291+
{
3292+
TkappObject *self = (TkappObject *)op;
3293+
Py_CLEAR(self->trace);
3294+
return 0;
3295+
}
3296+
3297+
static int
3298+
Tkapp_Traverse(PyObject *op, visitproc visit, void *arg)
3299+
{
3300+
TkappObject *self = (TkappObject *)op;
3301+
Py_VISIT(Py_TYPE(op));
3302+
Py_VISIT(self->trace);
3303+
return 0;
3304+
}
3305+
32603306
static void
32613307
Tkapp_Dealloc(PyObject *op)
32623308
{
3263-
TkappObject *self = TkappObject_CAST(op);
3264-
PyTypeObject *tp = Py_TYPE(self);
3309+
TkappObject *self = (TkappObject *)op;
3310+
PyTypeObject *tp = Py_TYPE(op);
3311+
PyObject_GC_UnTrack(op);
32653312
if (self->threaded && self->thread_id != Tcl_GetCurrentThread()) {
32663313
/* Deleting the interpreter from another thread aborts the process
32673314
("Tcl_AsyncDelete: async handler deleted by the wrong thread").
@@ -3280,8 +3327,8 @@ Tkapp_Dealloc(PyObject *op)
32803327
Tcl_DeleteInterp(Tkapp_Interp(self));
32813328
LEAVE_TCL
32823329
}
3283-
Py_XDECREF(self->trace);
3284-
PyObject_Free(self);
3330+
(void)Tkapp_Clear(op);
3331+
tp->tp_free(op);
32853332
Py_DECREF(tp);
32863333
DisableEventHook();
32873334
}
@@ -3488,6 +3535,8 @@ static PyMethodDef Tktt_methods[] =
34883535

34893536
static PyType_Slot Tktt_Type_slots[] = {
34903537
{Py_tp_dealloc, Tktt_Dealloc},
3538+
{Py_tp_traverse, Tktt_Traverse},
3539+
{Py_tp_clear, Tktt_Clear},
34913540
{Py_tp_repr, Tktt_Repr},
34923541
{Py_tp_methods, Tktt_methods},
34933542
{0, 0}
@@ -3500,6 +3549,7 @@ static PyType_Spec Tktt_Type_spec = {
35003549
Py_TPFLAGS_DEFAULT
35013550
| Py_TPFLAGS_DISALLOW_INSTANTIATION
35023551
| Py_TPFLAGS_IMMUTABLETYPE
3552+
| Py_TPFLAGS_HAVE_GC
35033553
),
35043554
.slots = Tktt_Type_slots,
35053555
};
@@ -3547,6 +3597,8 @@ static PyMethodDef Tkapp_methods[] =
35473597

35483598
static PyType_Slot Tkapp_Type_slots[] = {
35493599
{Py_tp_dealloc, Tkapp_Dealloc},
3600+
{Py_tp_traverse, Tkapp_Traverse},
3601+
{Py_tp_clear, Tkapp_Clear},
35503602
{Py_tp_methods, Tkapp_methods},
35513603
{0, 0}
35523604
};
@@ -3559,6 +3611,7 @@ static PyType_Spec Tkapp_Type_spec = {
35593611
Py_TPFLAGS_DEFAULT
35603612
| Py_TPFLAGS_DISALLOW_INSTANTIATION
35613613
| Py_TPFLAGS_IMMUTABLETYPE
3614+
| Py_TPFLAGS_HAVE_GC
35623615
),
35633616
.slots = Tkapp_Type_slots,
35643617
};

0 commit comments

Comments
 (0)