Skip to content

test_curses reference leak in the Python main branch #94644

Closed
Closed
test_curses reference leak in the Python main branch#94644
Assignees
kumaraditya303
Labels
3.12only security fixestype-bugAn unexpected behavior, bug, or error

Description

@vstinner
vstinner
opened on Jul 7, 2022

At commit e925241, test_curses leaks references:

$ ./python -m test test_curses -m test_output_string_embedded_null_chars -u all  -R 3:3
(...)
test_curses leaked [4, 4, 4] references, sum=12

I reproduce the issue on Fedora.

Example of failing buildbot: https://buildbot.python.org/all/#/builders/16/builds/244

Before this change, Python doesn't leak references.

Before: make:

gcc -fPIC -g -Og -Wall -O0 -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -DHAVE_NCURSESW=1 -I/usr/include/ncursesw -I./Include -I. -I/usr/local/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c /home/vstinner/python/main/Modules/_cursesmodule.c -o build/temp.linux-x86_64-3.12-pydebug/home/vstinner/python/main/Modules/_cursesmodule.o

gcc -fPIC -g -Og -Wall -O0 -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -DHAVE_NCURSESW=1 -I/usr/include/ncursesw -I./Include -I. -I/usr/local/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c /home/vstinner/python/main/Modules/_curses_panel.c -o build/temp.linux-x86_64-3.12-pydebug/home/vstinner/python/main/Modules/_curses_panel.o

After: make:

gcc -fPIC -g -Og -Wall -O0 -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -D_DEFAULT_SOURCE -I./Include -I. -I/usr/local/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c /home/vstinner/python/main/Modules/_cursesmodule.c -o build/temp.linux-x86_64-3.12-pydebug/home/vstinner/python/main/Modules/_cursesmodule.o

gcc -fPIC -g -Og -Wall -O0 -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -D_DEFAULT_SOURCE -I./Include -I. -I/usr/local/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c /home/vstinner/python/main/Modules/_curses_panel.c -o build/temp.linux-x86_64-3.12-pydebug/home/vstinner/python/main/Modules/_curses_panel.o

The commit removes -DHAVE_NCURSESW=1 -I/usr/include/ncursesw options and adds -D_DEFAULT_SOURCE option to the (gcc) build command line of the _curses extension.

cc @tiran @erlend-aasland

Activity

  1. vstinner
    added
    type-bugAn unexpected behavior, bug, or error
    on Jul 7, 2022

  2. tiran commented on Jul 7, 2022

    @tiran
    tiran
    on Jul 7, 2022
    Member

    HAVE_NCURSESW should be added to pyconfig.h. The pkg-config branch does not define the config value.

    The problem may have revealed an old bug in the non-wide curses build.

  3. kumaraditya303
    added
    3.12only security fixes
    on Jul 7, 2022
  4. kumaraditya303
    self-assigned this
    on Jul 7, 2022
  5. tiran
    added 2 commits that reference this issue on Jul 7, 2022

    pythongh-94644: Define HAVE_NCURSESW in pkg-config branch

    47a25cc

    gh-94644: Define HAVE_NCURSESW in pkg-config branch (#94651)

    9c60b25
  6. miss-islington
    added 3 commits that reference this issue on Jul 7, 2022

    GH-94644: fix test_curses ref leak (GH-94647)

    277f55c

    pythonGH-94644: fix test_curses ref leak (pythonGH-94647)

    712db4f

    pythonGH-94644: fix test_curses ref leak (pythonGH-94647)

    52992ad

  7. tiran commented on Jul 7, 2022

    @tiran
    tiran
    on Jul 7, 2022
    Member

    @kumaraditya303 fixed the reference leak when the curses module is built without ncursesw. I addressed the missing HAVE_NCURSESW in the pkg-config branch. Additional defines and include directories are now coming from pkg-config or env var overrides.

  8. miss-islington
    added a commit that references this issue on Jul 7, 2022

    GH-94644: fix test_curses ref leak (GH-94647)

    c5819c1

  9. vstinner commented on Jul 7, 2022

    @vstinner
    vstinner
    on Jul 7, 2022
    MemberAuthor

    I addressed the missing HAVE_NCURSESW in the pkg-config branch.

    What is this branch?

  10. tiran commented on Jul 7, 2022

    @tiran
    tiran
    on Jul 7, 2022
    Member

    The TRUE branch in PKG_CHECK_MODULES call:

        PKG_CHECK_MODULES([CURSES], [ncursesw], [
      AC_DEFINE([HAVE_NCURSESW], [1]) <<<<<<<<<<<<<<<<<<<< this line was missing
      have_curses=ncursesw
    ], [
      WITH_SAVE_ENV([
        AC_CHECK_LIB([ncursesw], [initscr], [
          AC_DEFINE([HAVE_NCURSESW], [1])
          have_curses=ncursesw
          CURSES_CFLAGS=${CURSES_CFLAGS-""}
          CURSES_LIBS=${CURSES_LIBS-"-lncursesw"}
        ])
      ])
    ])
  11. miss-islington
    added a commit that references this issue on Jul 7, 2022

    GH-94644: fix test_curses ref leak (GH-94647)

    77a0153
  12. tiran
    closed this as completedon Jul 7, 2022

  13. vstinner commented on Jul 7, 2022

    @vstinner
    vstinner
    on Jul 7, 2022
    MemberAuthor

    At commit b6558d7, the HAVE_NCURSESW macro is now defined as 1 in pyconfig.h:

    /* Define to 1 if you have the `ncursesw' library. */
#define HAVE_NCURSESW 1

    Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.12only security fixestype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions