This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Python leaks one reference at exit on Windows
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jeremy.kloth, jkloth, vstinner
Priority: normal Keywords: patch

Created on 2022-02-25 16:57 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 31589 merged vstinner, 2022-02-25 17:02
PR 31594 merged vstinner, 2022-02-26 01:41
Messages (14)
msg414020 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-25 16:57
"./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.
msg414025 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-25 17:24
New changeset ea9612a17bc60d44e0058f525d3c02a91c439cef by Victor Stinner in branch 'main':
bpo-46857: Fix test_embed.test_no_memleak() on Windows (GH-31589)
https://github.com/python/cpython/commit/ea9612a17bc60d44e0058f525d3c02a91c439cef
msg414056 - (view) Author: Jeremy Kloth (jkloth) * Date: 2022-02-26 01:29
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.
msg414057 - (view) Author: Jeremy Kloth (jkloth) * Date: 2022-02-26 01:38
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)
msg414058 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-26 01:50
> 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 GH-31594 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]
msg414060 - (view) Author: Jeremy Kloth (jeremy.kloth) Date: 2022-02-26 03:18
> 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 GH-31594 to fix this macro.

Even using that change, I still have negative refs (but I still have
Py_TRACE_REFS defined)
msg414062 - (view) Author: Jeremy Kloth (jeremy.kloth) Date: 2022-02-26 03:33
> ./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 GH-31594 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.
msg414123 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-26 23:28
New changeset ad56919c5ed54523f866e6605a2573ab7b7d5235 by Victor Stinner in branch 'main':
bpo-46857: Fix refleak in OSError INIT_ALIAS() (GH-31594)
https://github.com/python/cpython/commit/ad56919c5ed54523f866e6605a2573ab7b7d5235
msg414124 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-26 23:31
> 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 :-)
msg414125 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-26 23:34
I just built Python with --with-trace-refs. On Linux, it works as expected:

$ ./python -I -X showrefcount -c pass 
[0 refs, 0 blocks]
msg414128 - (view) Author: Jeremy Kloth (jeremy.kloth) Date: 2022-02-26 23:52
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
msg414130 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-27 00:11
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.
msg414492 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-03-03 23:38
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.
msg415765 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-03-22 12:55
The last leak of a memory block on Windows was fixed by:

New changeset 88872a29f19092d2fde27365af230abd6d301941 by Jeremy Kloth in branch 'main':
bpo-47084: Clear Unicode cached representations on finalization (GH-32032)
https://github.com/python/cpython/commit/88872a29f19092d2fde27365af230abd6d301941
History
Date User Action Args
2022-04-11 14:59:56adminsetgithub: 91013
2022-03-22 12:55:03vstinnersetmessages: + msg415765
2022-03-03 23:38:23vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg414492

stage: patch review -> resolved
2022-02-27 00:11:40vstinnersetmessages: + msg414130
2022-02-26 23:52:08jeremy.klothsetmessages: + msg414128
2022-02-26 23:34:34vstinnersetmessages: + msg414125
2022-02-26 23:31:59vstinnersetmessages: + msg414124
2022-02-26 23:28:44vstinnersetmessages: + msg414123
2022-02-26 03:33:09jeremy.klothsetmessages: + msg414062
2022-02-26 03:18:28jeremy.klothsetnosy: + jeremy.kloth
messages: + msg414060
2022-02-26 01:50:52vstinnersetmessages: + msg414058
2022-02-26 01:41:34vstinnersetpull_requests: + pull_request29717
2022-02-26 01:38:00jklothsetmessages: + msg414057
2022-02-26 01:29:38jklothsetnosy: + jkloth
messages: + msg414056
2022-02-25 17:24:41vstinnersetmessages: + msg414025
2022-02-25 17:02:49vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request29712
2022-02-25 16:57:11vstinnercreate