Skip to content

Multiple reference leaks in curses error-branches  #123290

Closed

Description

@picnixz

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:

m = PyModule_Create(&_cursesmodule);

There are also calls that are not checked against errors:

/* 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

Activity

  1. self-assigned this
    on Aug 24, 2024
  2. changed the title [-]Multiple error checks branches leak in `curses` [/-] [+]Multiple reference leaks in `curses` [/+] on Sep 3, 2024
  3. added a commit that references this issue on Sep 11, 2024
  4. changed the title [-]Multiple reference leaks in `curses` [/-] [+]Multiple reference leaks in `curses` error-branches [/+] on Sep 11, 2024
  5. picnixz commented on Sep 11, 2024

    @picnixz
    MemberAuthor

    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).

  6. vstinner commented on Sep 12, 2024

    @vstinner
    Member

    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.

  7. added a commit that references this issue on Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

extension-modulesC modules in the Modules dirtype-bugAn unexpected behavior, bug, or error

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions