From 90a186a65826bffbc46c95f9482a2fa625f59bd8 Mon Sep 17 00:00:00 2001 From: Michael Forney Date: Tue, 18 May 2021 19:03:32 -0700 Subject: [PATCH 1/4] bpo-44172: Keep reference to original window in curses subwindow objects The X/Open curses specification[0] and ncurses documentation[1] both state that subwindows must be deleted before the main window. Deleting the windows in the wrong order causes a double-free with NetBSD's curses implementation. To fix this, keep track of the original window object in the subwindow object, and keep a reference to the original for the lifetime of the subwindow. [0] https://pubs.opengroup.org/onlinepubs/7908799/xcurses/delwin.html [1] https://invisible-island.net/ncurses/man/curs_window.3x.html --- Include/py_curses.h | 3 ++- .../2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst | 3 +++ Modules/_cursesmodule.c | 20 +++++++++++-------- 3 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst diff --git a/Include/py_curses.h b/Include/py_curses.h index b70252d9d7605e..83b6934692bd8a 100644 --- a/Include/py_curses.h +++ b/Include/py_curses.h @@ -58,10 +58,11 @@ extern "C" { /* Type declarations */ -typedef struct { +typedef struct PyCursesWindowObject { PyObject_HEAD WINDOW *win; char *encoding; + struct PyCursesWindowObject *orig; } PyCursesWindowObject; #define PyCursesWindow_Check(v) Py_IS_TYPE(v, &PyCursesWindow_Type) diff --git a/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst b/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst new file mode 100644 index 00000000000000..68af42a807f049 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst @@ -0,0 +1,3 @@ +Keep a reference to original PyCursesWindowObject in subwindows so +that the original WINDOW does not get deleted before the subwindow +WINDOW. diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index b6b7ca458b454a..249669b8cea2b1 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -663,7 +663,8 @@ Window_TwoArgNoReturnFunction(wresize, int, "ii;lines,columns") /* Allocation and deallocation of Window Objects */ static PyObject * -PyCursesWindow_New(WINDOW *win, const char *encoding) +PyCursesWindow_New(WINDOW *win, const char *encoding, + PyCursesWindowObject *orig) { PyCursesWindowObject *wo; @@ -694,6 +695,8 @@ PyCursesWindow_New(WINDOW *win, const char *encoding) PyErr_NoMemory(); return NULL; } + wo->orig = orig; + Py_XINCREF(orig); return (PyObject *)wo; } @@ -703,6 +706,7 @@ PyCursesWindow_Dealloc(PyCursesWindowObject *wo) if (wo->win != stdscr) delwin(wo->win); if (wo->encoding != NULL) PyMem_Free(wo->encoding); + Py_XDECREF(wo->orig); PyObject_Free(wo); } @@ -1304,7 +1308,7 @@ _curses_window_derwin_impl(PyCursesWindowObject *self, int group_left_1, return NULL; } - return (PyObject *)PyCursesWindow_New(win, NULL); + return (PyObject *)PyCursesWindow_New(win, NULL, self); } /*[clinic input] @@ -2332,7 +2336,7 @@ _curses_window_subwin_impl(PyCursesWindowObject *self, int group_left_1, return NULL; } - return (PyObject *)PyCursesWindow_New(win, self->encoding); + return (PyObject *)PyCursesWindow_New(win, self->encoding, self); } /*[clinic input] @@ -3081,7 +3085,7 @@ _curses_getwin(PyObject *module, PyObject *file) PyErr_SetString(PyCursesError, catchall_NULL); goto error; } - res = PyCursesWindow_New(win, NULL); + res = PyCursesWindow_New(win, NULL, NULL); error: fclose(fp); @@ -3254,7 +3258,7 @@ _curses_initscr_impl(PyObject *module) if (initialised) { wrefresh(stdscr); - return (PyObject *)PyCursesWindow_New(stdscr, NULL); + return (PyObject *)PyCursesWindow_New(stdscr, NULL, NULL); } win = initscr(); @@ -3346,7 +3350,7 @@ _curses_initscr_impl(PyObject *module) SetDictInt("LINES", LINES); SetDictInt("COLS", COLS); - winobj = (PyCursesWindowObject *)PyCursesWindow_New(win, NULL); + winobj = (PyCursesWindowObject *)PyCursesWindow_New(win, NULL, NULL); screen_encoding = winobj->encoding; return (PyObject *)winobj; } @@ -3719,7 +3723,7 @@ _curses_newpad_impl(PyObject *module, int nlines, int ncols) return NULL; } - return (PyObject *)PyCursesWindow_New(win, NULL); + return (PyObject *)PyCursesWindow_New(win, NULL, NULL); } /*[clinic input] @@ -3758,7 +3762,7 @@ _curses_newwin_impl(PyObject *module, int nlines, int ncols, return NULL; } - return (PyObject *)PyCursesWindow_New(win, NULL); + return (PyObject *)PyCursesWindow_New(win, NULL, NULL); } /*[clinic input] From b04218b4b15c7b59f2c6af251b5ed3d0c8bae785 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 12 Apr 2025 23:15:21 +0300 Subject: [PATCH 2/4] Add a test. --- Lib/test/test_curses.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_curses.py b/Lib/test/test_curses.py index cc3aa561cd4c42..116112f9feb6bc 100644 --- a/Lib/test/test_curses.py +++ b/Lib/test/test_curses.py @@ -8,7 +8,8 @@ from unittest.mock import MagicMock from test.support import (requires, verbose, SaveSignals, cpython_only, - check_disallow_instantiation, MISSING_C_DOCSTRINGS) + check_disallow_instantiation, MISSING_C_DOCSTRINGS, + gc_collect) from test.support.import_helper import import_module # Optionally test curses module. This currently requires that the @@ -181,6 +182,14 @@ def test_create_windows(self): self.assertEqual(win3.getparyx(), (2, 1)) self.assertEqual(win3.getmaxyx(), (6, 11)) + def test_subwindows_references(self): + win = curses.newwin(5, 10) + win2 = win.subwin(3, 7) + del win + gc_collect() + del win2 + gc_collect() + def test_move_cursor(self): stdscr = self.stdscr win = stdscr.subwin(10, 15, 2, 5) From fac2acc17e030442011473721245d7640d51c05f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 12 Apr 2025 23:17:53 +0300 Subject: [PATCH 3/4] Add visitor in tp_traverse. --- Modules/_cursesmodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 0b9f7cfb19197c..6da3ab0fe251f1 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -850,6 +850,8 @@ static int PyCursesWindow_traverse(PyObject *self, visitproc visit, void *arg) { Py_VISIT(Py_TYPE(self)); + PyCursesWindowObject *wo = (PyCursesWindowObject *)self; + Py_VISIT(wo->orig); return 0; } From 0478d171c4f223502137ebb8bad03d5ee1d6bfed Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 12 Apr 2025 23:26:57 +0300 Subject: [PATCH 4/4] Update the NEWS entry. --- .../next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst b/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst index 68af42a807f049..d53f3725100eb2 100644 --- a/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst +++ b/Misc/NEWS.d/next/Library/2021-05-18-19-12-58.bpo-44172.rJ_-CI.rst @@ -1,3 +1,2 @@ -Keep a reference to original PyCursesWindowObject in subwindows so -that the original WINDOW does not get deleted before the subwindow -WINDOW. +Keep a reference to original :mod:`curses` windows in subwindows so +that the original window does not get deleted before subwindows.