Repository navigation
Multiple reference leaks in curses error-branches #123290
Description
Bug report
Bug description:
In _cursesmodule.c, we have
#define SetDictInt(string,ch) \
do { \
PyObject *o = PyLong_FromLong((long) (ch)); \
if (o && PyDict_SetItemString(ModDict, string, o) == 0) { \
Py_DECREF(o); \
} \
} while (0)However, PyDict_SetItemString does not steal a reference. If the dict insertion fails for whatever reason, o must be decref, which is not the case here. Similarly, in PyInit__curses, a module object is explicitly created via PyModule_Create but is not decref if an error occurs:
cpython/Modules/_cursesmodule.c
Line 4766 in b178bee
| m = PyModule_Create(&_cursesmodule); |
There are also calls that are not checked against errors:
cpython/Modules/_cursesmodule.c
Lines 4808 to 4811 in b178bee
| /* Make the version available */ | |
| v = PyBytes_FromString(PyCursesVersion); | |
| PyDict_SetItemString(d, "version", v); | |
| PyDict_SetItemString(d, "__version__", v); |
Here, insertion is assumed to always succeed, but this may not be the case, in which case we shouldn't continue.
On the other hand, the PyCursesWindowType leaks since it's never cleared. While it does not matter if you exit the interpreter, I think it may matter if you just want to unload the module and continue with a normal execution.
There are also other bugs that are more generally related to error-handling where some pointers might be NULL but are used as if they were not NULL, e.g. #123913).
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
[-]Multiple error checks branches leak in `curses` [/-][+]Multiple reference leaks in `curses` [/+]gh-123290: fix reference leaks in the error-branches of `_cursesmodul…
[-]Multiple reference leaks in `curses` [/-][+]Multiple reference leaks in `curses` error-branches [/+]picnixz commentedon Sep 11, 2024
I'll keep the issue opened until we decide whether to backport the changes or not one by one, or in one go (there will be subsequent modifications on the curses module).
vstinner commentedon Sep 12, 2024
The code is old and nobody reported issues about ref leaks. IMO the change is too big to be backported. I don't think that it's worth it to backport it. I close the issue.
gh-123290: Fix decref in _curses update_lines_cols() (#124767)