Skip to content

Python leaks one reference at exit on Windows #91013

Closed
Closed
Python leaks one reference at exit on Windows#91013
Labels
3.11only security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)

Description

@vstinner
vstinner
opened on Feb 25, 2022
BPO 46857
Nosy @vstinner, @jkloth
PRs
  • bpo-46857: Fix test_embed.test_no_memleak() on Windows #31589
  • bpo-46857: Fix refleak in OSError INIT_ALIAS() #31594
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2022-03-03.23:38:23.521>
created_at = <Date 2022-02-25.16:57:11.797>
labels = ['interpreter-core', '3.11']
title = 'Python leaks one reference at exit on Windows'
updated_at = <Date 2022-03-22.12:55:03.311>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-03-22.12:55:03.311>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-03.23:38:23.521>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2022-02-25.16:57:11.797>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 46857
keywords = ['patch']
message_count = 14.0
messages = ['414020', '414025', '414056', '414057', '414058', '414060', '414062', '414123', '414124', '414125', '414128', '414130', '414492', '415765']
nosy_count = 3.0
nosy_names = ['vstinner', 'jkloth', 'jeremy.kloth']
pr_nums = ['31589', '31594']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46857'
versions = ['Python 3.11']

    Activity

    1. vstinner commented on Feb 25, 2022

      @vstinner
      vstinner
      on Feb 25, 2022
      MemberAuthor

      "./python -X showrefcount -I -c pass" returns "[0 refs, 0 blocks]" as expected on Linux: Python doesn't leak any reference nor memory block.

      But on Windows, it still leaks 1 reference (and 1 memory block)!

      vstinner@DESKTOP-DK7VBIL C:\vstinner\python\main>python -X showrefcount -I -c pass
      [1 refs, 1 blocks]

      I recently added a test in test_embed which now fails on Windows.

      See bpo-1635741 "Py_Finalize() doesn't clear all Python objects at exit" for the context.

    2. vstinner
      added
      3.11only security fixes
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Feb 25, 2022

    3. vstinner commented on Feb 25, 2022

      @vstinner
      vstinner
      on Feb 25, 2022
      MemberAuthor

      New changeset ea9612a by Victor Stinner in branch 'main':
      bpo-46857: Fix test_embed.test_no_memleak() on Windows (GH-31589)
      ea9612a

    4. jkloth commented on Feb 26, 2022

      @jkloth
      jkloth
      on Feb 26, 2022
      Contributor

      Good news, the difference on Windows was easy enough to find, bad news total refs are now negative!

      --- a/Objects/exceptions.c
      +++ b/Objects/exceptions.c
      @@ -3647,8 +3647,7 @@ _PyBuiltins_AddExceptions(PyObject *bltinmod)

       #define INIT_ALIAS(NAME, TYPE) \
     do { \
-        Py_INCREF(PyExc_ ## TYPE); \
-        Py_XDECREF(PyExc_ ## NAME); \
+        Py_XSETREF(PyExc_ ## NAME, PyExc_ ## TYPE); \
         PyExc_ ## NAME = PyExc_ ## TYPE; \
         if (PyDict_SetItemString(mod_dict, # NAME, PyExc_ ## NAME)) { \
             return -1; \

      As the PyExc_* aliases just deprecated names for PyExc_OSError, there is no need to increment their refcounts. Or they could be decremented in Fini(). Or they could finally be removed entirely.

    5. jkloth commented on Feb 26, 2022

      @jkloth
      jkloth
      on Feb 26, 2022
      Contributor

      Note that an allocated block is still leaking.

      Strange as well, when using dump_refs, the total refs are much more negative (-12 linux, -13 Windows)

    6. vstinner commented on Feb 26, 2022

      @vstinner
      vstinner
      on Feb 26, 2022
      MemberAuthor

      Good news, the difference on Windows was easy enough to find, bad news total refs are now negative!

      Oh wow. How did you find this leak? Did you read all C files and check for code specific to Windows? How did you proceed? Well spotted!

      #define INIT_ALIAS(NAME, TYPE)

      I proposed #75775 to fix this macro.

      Strange as well, when using dump_refs, the total refs are much more negative (-12 linux, -13 Windows)

      Which command do you type? Do you pass -I option to Python?

      With my PR, I get exactly 0 on Linux:

      $ ./python -I -X showrefcount -c pass
[0 refs, 0 blocks]

      Note that an allocated block is still leaking.

      Right, with my PR, I now get 1 leaked memory block on Windows:

      python -I -X showrefcount -c pass
      [0 refs, 1 blocks]

    7. jeremykloth commented on Feb 26, 2022

      @jeremykloth
      jeremyklothmannequin
      on Feb 26, 2022
      Mannequin

      Oh wow. How did you find this leak? Did you read all C files and check for code specific to Windows? How did you proceed? Well spotted!

      Initially, I modified Py_INCREF to dump the object (addr & tp_name) on
      initial inc (ob_refcnt == 1) and Py_DECREF to dump on final dec
      (ob_refcnt == 0).
      Then filter that list (~65K) to find objects not dealloc'ed. Given
      those names (~200), cross-check with source files containing 'ifdef
      MS_WINDOWS' (and related spellings).

      Which command do you type? Do you pass -I option to Python?

      For both as -I disables environment lookup:
--- a/Python/initconfig.c
+++ b/Python/initconfig.c
@@ -757,6 +757,7 @@ config_init_defaults(PyConfig *config)
    config->user_site_directory = 1;
    config->buffered_stdio = 1;
    config->pathconfig_warnings = 1;
+   config->dump_refs = 1;
#ifdef MS_WINDOWS
    config->legacy_windows_stdio = 0;
#endif

      For linux:
      ./configure --enabled-shared --with-py-debug --with-trace-refs
      make build_all
      LD_LIBRARY_PATH=$PWD ./python -X showrefcount -I -c pass

      For Windows:
      Add "#define Py_TRACE_REFS 1" to PC\pyconfig.h
      build.bat -d -e
      amd64\python_d.exe -X showrefcount -I -c pass

      I proposed #75775 to fix this macro.

      Even using that change, I still have negative refs (but I still have
      Py_TRACE_REFS defined)

    8. jeremykloth commented on Feb 26, 2022

      @jeremykloth
      jeremyklothmannequin
      on Feb 26, 2022
      Mannequin

      ./configure --enabled-shared --with-py-debug --with-trace-refs

      (that's what I get for typing from memory):
      ./configure --enable-shared --with-pydebug --with-trace-refs

      > I proposed #75775 to fix this macro.

      Even using that change, I still have negative refs (but I still have
      Py_TRACE_REFS defined)

      I initially missed the _PySet_Dummy change, with that total refs (w/o
      dump_refs) is now 0.

    9. vstinner commented on Feb 26, 2022

      @vstinner
      vstinner
      on Feb 26, 2022
      MemberAuthor

      New changeset ad56919 by Victor Stinner in branch 'main':
      bpo-46857: Fix refleak in OSError INIT_ALIAS() (GH-31594)
      ad56919

    10. vstinner commented on Feb 26, 2022

      @vstinner
      vstinner
      on Feb 26, 2022
      MemberAuthor

      Initially, I modified Py_INCREF to dump the object (addr & tp_name) on
      initial inc (ob_refcnt == 1) and Py_DECREF to dump on final dec
      (ob_refcnt == 0). Then filter that list (~65K) to find objects not
      dealloc'ed. Given those names (~200), cross-check with source files
      containing 'ifdef MS_WINDOWS' (and related spellings).

      That's smart! Thanks for sharing. It may help to identify future leaks.

      Even using that change, I still have negative refs (but I still have
      Py_TRACE_REFS defined)

      Ah, maybe testing with Py_TRACE_REFS shows more bugs. I didn't try Py_TRACE_REFS recently. Well, if someone finds why it's becoming negative, a fix would be welcomed :-)

    11. vstinner commented on Feb 26, 2022

      @vstinner
      vstinner
      on Feb 26, 2022
      MemberAuthor

      I just built Python with --with-trace-refs. On Linux, it works as expected:

      $ ./python -I -X showrefcount -c pass 
[0 refs, 0 blocks]

    12. jeremykloth commented on Feb 26, 2022

      @jeremykloth
      jeremyklothmannequin
      on Feb 26, 2022
      Mannequin

      Did you also modify initconfig.c? That part is required as the usual
      processing of the environment variable PYTHONDUMPREFS needed to enable
      tracing output is ignored with -I

    13. vstinner commented on Feb 27, 2022

      @vstinner
      vstinner
      on Feb 27, 2022
      MemberAuthor

      Ah, with PYTHONDUMPREFS=1 (and without -I), I get a negative ref count:

      $ PYTHONDUMPREFS=1 ./python -X showrefcount -c pass
[-10 refs, 0 blocks]

      I don't plan to investigate this issue. I'm not using PYTHONDUMPREFS=1 anymore.

    14. vstinner commented on Mar 3, 2022

      @vstinner
      vstinner
      on Mar 3, 2022
      MemberAuthor

      The initial issue "Python leaks one reference at exit on Windows" is now fixed.

      If someone wants to investigate the remaining leak of 1 memory block or the negative ref count of PYTHONDUMPREFS=1, please open a separated issue.

    15. vstinner
      closed this as completedon Mar 3, 2022
    16. vstinner
      closed this as completedon Mar 3, 2022

    17. vstinner commented on Mar 22, 2022

      @vstinner
      vstinner
      on Mar 22, 2022
      MemberAuthor

      The last leak of a memory block on Windows was fixed by:

      New changeset 88872a2 by Jeremy Kloth in branch 'main':
      bpo-47084: Clear Unicode cached representations on finalization (GH-32032)
      88872a2

    18. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.11only security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions