From 93694c87f11fb1d7ccfd27f5b115c1abfb406558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 1 Sep 2025 12:10:52 +0200 Subject: [PATCH 1/8] fully implement GC protocol for `_curses_panel.panel` --- Modules/_curses_panel.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 3408a505575580..211214954c051e 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -410,8 +410,11 @@ static PyObject * PyCursesPanel_New(_curses_panel_state *state, PANEL *pan, PyCursesWindowObject *wo) { - PyCursesPanelObject *po = PyObject_New(PyCursesPanelObject, - state->PyCursesPanel_Type); + assert(state != NULL); + PyTypeObject *type = state->PyCursesPanel_Type; + assert(type != NULL); + assert(type->tp_alloc != NULL); + PyCursesPanelObject *po = (PyCursesPanelObject *)type->tp_alloc(type, 0); if (po == NULL) { return NULL; } @@ -429,11 +432,11 @@ PyCursesPanel_New(_curses_panel_state *state, PANEL *pan, static void PyCursesPanel_Dealloc(PyObject *self) { - PyObject *tp, *obj; - PyCursesPanelObject *po = _PyCursesPanelObject_CAST(self); + PyTypeObject *tp = Py_TYPE(self); + PyObject_GC_UnTrack(self); - tp = (PyObject *) Py_TYPE(po); - obj = (PyObject *) panel_userptr(po->pan); + PyCursesPanelObject *po = _PyCursesPanelObject_CAST(self); + PyObject *obj = (PyObject *)panel_userptr(po->pan); if (obj) { Py_DECREF(obj); if (set_panel_userptr(po->pan, NULL) == ERR) { @@ -452,10 +455,19 @@ PyCursesPanel_Dealloc(PyObject *self) PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()"); } } - PyObject_Free(po); + tp->tp_free(po); Py_DECREF(tp); } +static int +PyCursesPanel_Traverse(PyObject *op, visitproc visit, void *arg) +{ + PyCursesPanelObject *self = _PyCursesPanelObject_CAST(op); + Py_VISIT(Py_TYPE(op)); + Py_VISIT(self->wo); + return 0; +} + /* panel_above(NULL) returns the bottom panel in the stack. To get this behaviour we use curses.panel.bottom_panel(). */ /*[clinic input] @@ -648,6 +660,7 @@ static PyMethodDef PyCursesPanel_Methods[] = { static PyType_Slot PyCursesPanel_Type_slots[] = { {Py_tp_dealloc, PyCursesPanel_Dealloc}, + {Py_tp_traverse, PyCursesPanel_Traverse}, {Py_tp_methods, PyCursesPanel_Methods}, {0, 0}, }; @@ -655,7 +668,11 @@ static PyType_Slot PyCursesPanel_Type_slots[] = { static PyType_Spec PyCursesPanel_Type_spec = { .name = "_curses_panel.panel", .basicsize = sizeof(PyCursesPanelObject), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION, + .flags = ( + Py_TPFLAGS_DEFAULT + | Py_TPFLAGS_DISALLOW_INSTANTIATION + | Py_TPFLAGS_HAVE_GC + ), .slots = PyCursesPanel_Type_slots }; From c05db69e8e68b725b568cdf0c4f0db68e8880c0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:15:35 +0200 Subject: [PATCH 2/8] address review comment --- Modules/_curses_panel.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 211214954c051e..fdb5bf3d482e28 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -429,6 +429,23 @@ PyCursesPanel_New(_curses_panel_state *state, PANEL *pan, return (PyObject *)po; } +static int +PyCursesPanel_Clear(PyObject *op) +{ + PyCursesPanelObject *self = _PyCursesPanelObject_CAST(op); + PyObject *extra = (PyObject *)panel_userptr(self->pan); + if (extra != NULL) { + Py_CLEAR(extra); + if (set_panel_userptr(self->pan, NULL) == ERR) { + curses_panel_panel_set_error(self, "set_panel_userptr", NULL); + // Do not add a PyErr_FormatUnraisable() because the GC + // is responsible for handling exceptions in tp_clear. + } + } + // do NOT clear self->wo yet as there is no cycle to break with it + return 0; +} + static void PyCursesPanel_Dealloc(PyObject *self) { @@ -464,6 +481,7 @@ PyCursesPanel_Traverse(PyObject *op, visitproc visit, void *arg) { PyCursesPanelObject *self = _PyCursesPanelObject_CAST(op); Py_VISIT(Py_TYPE(op)); + Py_VISIT(panel_userptr(self->pan)); Py_VISIT(self->wo); return 0; } @@ -659,6 +677,7 @@ static PyMethodDef PyCursesPanel_Methods[] = { /* -------------------------------------------------------*/ static PyType_Slot PyCursesPanel_Type_slots[] = { + {Py_tp_clear, PyCursesPanel_Clear}, {Py_tp_dealloc, PyCursesPanel_Dealloc}, {Py_tp_traverse, PyCursesPanel_Traverse}, {Py_tp_methods, PyCursesPanel_Methods}, From 23a966428b60ed5a0d030824dace2a5045b5c5d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:20:29 +0200 Subject: [PATCH 3/8] explicitly raise PyErr_FormatUnraisable in `tp_clear` --- Modules/_curses_panel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index fdb5bf3d482e28..09f556fd591b1c 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -438,8 +438,7 @@ PyCursesPanel_Clear(PyObject *op) Py_CLEAR(extra); if (set_panel_userptr(self->pan, NULL) == ERR) { curses_panel_panel_set_error(self, "set_panel_userptr", NULL); - // Do not add a PyErr_FormatUnraisable() because the GC - // is responsible for handling exceptions in tp_clear. + PyErr_FormatUnraisable("Exception ignored in tp_clear of %s", op); } } // do NOT clear self->wo yet as there is no cycle to break with it From 13933a97f6c4dc06c1115c684392d3872bd72590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:23:50 +0200 Subject: [PATCH 4/8] explicitly raise PyErr_FormatUnraisable in `tp_clear` --- Modules/_curses_panel.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 09f556fd591b1c..75b5f1edaad4fc 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -452,14 +452,7 @@ PyCursesPanel_Dealloc(PyObject *self) PyObject_GC_UnTrack(self); PyCursesPanelObject *po = _PyCursesPanelObject_CAST(self); - PyObject *obj = (PyObject *)panel_userptr(po->pan); - if (obj) { - Py_DECREF(obj); - if (set_panel_userptr(po->pan, NULL) == ERR) { - curses_panel_panel_set_error(po, "set_panel_userptr", "__del__"); - PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()"); - } - } + (void)PyCursesPanel_Clear(self); if (del_panel(po->pan) == ERR && !PyErr_Occurred()) { curses_panel_panel_set_error(po, "del_panel", "__del__"); PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()"); From bfadf5bbf997a6b8a4a52d119337d5a1eb416163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:24:52 +0200 Subject: [PATCH 5/8] fixup --- Modules/_curses_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 75b5f1edaad4fc..8533c75fcdbef0 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -438,7 +438,7 @@ PyCursesPanel_Clear(PyObject *op) Py_CLEAR(extra); if (set_panel_userptr(self->pan, NULL) == ERR) { curses_panel_panel_set_error(self, "set_panel_userptr", NULL); - PyErr_FormatUnraisable("Exception ignored in tp_clear of %s", op); + PyErr_FormatUnraisable("Exception ignored in tp_clear of %T", op); } } // do NOT clear self->wo yet as there is no cycle to break with it From 49cf9ea7f2b212bd4778805f6eb1f5241cbe8232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:20:51 +0200 Subject: [PATCH 6/8] address review --- Modules/_curses_panel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 8533c75fcdbef0..33875ea71ccdf8 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -438,10 +438,9 @@ PyCursesPanel_Clear(PyObject *op) Py_CLEAR(extra); if (set_panel_userptr(self->pan, NULL) == ERR) { curses_panel_panel_set_error(self, "set_panel_userptr", NULL); - PyErr_FormatUnraisable("Exception ignored in tp_clear of %T", op); } } - // do NOT clear self->wo yet as there is no cycle to break with it + // self->wo should not be cleared because an associated WINDOW may exist return 0; } From 32211279a5f78fb44c7868645bcdbf90274e96f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:23:48 +0200 Subject: [PATCH 7/8] use Py_DECREF when possible --- Modules/_curses_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 33875ea71ccdf8..53743ab195243e 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -435,7 +435,7 @@ PyCursesPanel_Clear(PyObject *op) PyCursesPanelObject *self = _PyCursesPanelObject_CAST(op); PyObject *extra = (PyObject *)panel_userptr(self->pan); if (extra != NULL) { - Py_CLEAR(extra); + Py_DECREF(extra); if (set_panel_userptr(self->pan, NULL) == ERR) { curses_panel_panel_set_error(self, "set_panel_userptr", NULL); } From 39de85bf065e50d1f17a1344c0a9581a4ffd8e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 2 Sep 2025 14:58:32 +0200 Subject: [PATCH 8/8] handle errors from `tp_clear` in `tp_dealloc` --- Modules/_curses_panel.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 53743ab195243e..66a8c40953da8c 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -438,6 +438,7 @@ PyCursesPanel_Clear(PyObject *op) Py_DECREF(extra); if (set_panel_userptr(self->pan, NULL) == ERR) { curses_panel_panel_set_error(self, "set_panel_userptr", NULL); + return -1; } } // self->wo should not be cleared because an associated WINDOW may exist @@ -451,7 +452,9 @@ PyCursesPanel_Dealloc(PyObject *self) PyObject_GC_UnTrack(self); PyCursesPanelObject *po = _PyCursesPanelObject_CAST(self); - (void)PyCursesPanel_Clear(self); + if (PyCursesPanel_Clear(self) < 0) { + PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()"); + } if (del_panel(po->pan) == ERR && !PyErr_Occurred()) { curses_panel_panel_set_error(po, "del_panel", "__del__"); PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()");