[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Mark Shannon]

Mark Shannon  added the comment:


New changeset 26b0482393a313e3bda364a35e7417e9db52c1c4 by Christian Heimes in 
branch 'main':
bpo-46476: Simplify and fix _PyStaticCode_Dealloc (GH-30965)
https://github.com/python/cpython/commit/26b0482393a313e3bda364a35e7417e9db52c1c4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Christian Heimes]

Change by Christian Heimes :


--
nosy: +christian.heimes
nosy_count: 6.0 -> 7.0
pull_requests: +29144
pull_request: https://github.com/python/cpython/pull/30965

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

Thanks Kumar! Your change fixed my issue:

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

Python no longer leaks memory at exit: it "leaks" exactly *zero* memory block.

The negative reference count is likely caused by bpo-46449.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:


New changeset c7f810b34d91a5c2fbe0a8385562015d2dd961f2 by Kumar Aditya in 
branch 'main':
bpo-46476: Fix memory leak in code objects generated by deepfreeze (GH-30853)
https://github.com/python/cpython/commit/c7f810b34d91a5c2fbe0a8385562015d2dd961f2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

In the end I was able to restore the codeobjects before they were quickened so 
it works for both embedded and python command now. See the latest commits and 
it is also ready for review now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

Created PR https://github.com/python/cpython/pull/30853

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Change by Kumar Aditya :


--
keywords: +patch
pull_requests: +29034
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/30853

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

Releasing memory in pymain_free() is a good start! At least it fix the issue 
for the "python" command. So people running a memory debugger like Valgrind 
will no longer see any leak at "python" exit.

Kumar: Do you want to work on a PR for that?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

> Aha. Maybe for now, the memory of immortal code objects can be deallocated in 
> pymain_free(). It's the last function called before Python exit the process.

It seems like a viable option for now to only clear them when python is not 
embedded and in pymain_free.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

> Kumar, I'm not sure I follow your concerns about the bootstrap working 
> differently on Windows than on Unix. Is the problem that on Unix the 
> bootstrap interpreter is linked without deepfreeze.c so there is no 
> definition of the symbol _Py_Deepfreeze_Fini? In that case, you can probably 
> just add a dummy one to _bootstrap_python.c.

Yes, that comment was outdated as I didn't knew that _bootstrap_python.c and 
_freeze_module.c are different executables on Linux. In the latest commit it is 
fixed and there are 0 memory blocks left on Linux and 131 on Windows. The next 
thing to be done is solving how to restore the code objects before it was 
quickened.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Guido van Rossum]

Guido van Rossum  added the comment:

> With the current design, it isn't possible though because the code objects 
> are modified in place so if co_quickened is freed the VM still tries to 
> execute the copied instructions.

Or the cleanup code could also restore co_firstinstr and other things that are 
set by quickened (the co_quickened flag and what else?).

Kumar, I'm not sure I follow your concerns about the bootstrap working 
differently on Windows than on Unix. Is the problem that on Unix the bootstrap 
interpreter is linked without deepfreeze.c so there is no definition of the 
symbol _Py_Deepfreeze_Fini? In that case, you can probably just add a dummy one 
to _bootstrap_python.c.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

Aha. Maybe for now, the memory of immortal code objects can be deallocated in 
pymain_free(). It's the last function called before Python exit the process.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

> Be careful, Python must remain usable after Py_Finalize(): it's legit to call 
> Py_Initialize() again and execute new Python code. Example executing the same 
> code 4 times, each time Py_Initialize() and Py_Finalize() are called:

./Programs/_testembed test_repeated_init_exec 'print("Hello")'

With the current design, it isn't possible though because the code objects are 
modified in place so if co_quickened is freed the VM still tries to execute the 
copied instructions.

See 
https://github.com/python/cpython/blob/76dc047a0e88d10aad0405228d56e94438cdd91c/Python/specialize.c#L425

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

FYI, I updated the build files and got it working on Windows and clears around 
~60 memory blocks. See the latest commit in branch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

> See branch https://github.com/kumaraditya303/cpython/commits/fix-code

Oh nice, I like this new _Py_Deepfreeze_Fini() function :-) I suggest to create 
a function specific to only clear immortal code objects. In my experience, it's 
important to control exactly when objects are cleared at Python exit: 
Py_Finalize() is complex and fragile. See my notes:
https://pythondev.readthedocs.io/finalization.html

Be careful, Python must remain usable after Py_Finalize(): it's legit to call 
Py_Initialize() again and execute new Python code. Example executing the same 
code 4 times, each time Py_Initialize() and Py_Finalize() are called:

./Programs/_testembed test_repeated_init_exec 'print("Hello")'

My _PyStaticMethod_Dealloc() implementation uses Py_CLEAR() rather than 
Py_XDECREF() to set structure members to NULL.

Moreover, there are more things than just co_quickened which should be cleared. 
I suggest to add a new function to clear an "immortal" code object. For 
example, I also suggest to call PyObject_ClearWeakRefs(). I guess that co_extra 
should also be cleared.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

bootstrap interpreter on Windows => bootstrap interpreter on Unix

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

Clearing co_quickened is easy, but it would requires changes to the build 
system to change the build order on Windows and bootstrap interpreter on 
Windows. I manually edited the Windows project files but it requires generating 
deepfreeze.c before hand. It cleared around ~60 memory blocks on Windows.

See branch https://github.com/kumaraditya303/cpython/commits/fix-code

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Kumar Aditya]

Kumar Aditya  added the comment:

> Would it be possible to enhance deepfreeze be produce a list of all 
> (immortal) code objects?

It is tricky because the deepfreeze modules are generated by the bootstrap 
interpreter in Linux/MacOS and the downloaded python from nuget interpreter on 
Windows so when the bootstrap interpreter is built there will be no list of 
code objects to begin with so it won't work as intended.
But I have an idea : If we can #define Py_DEEPFROZEN_MODULES in the final 
interpreter but not in the bootstrap one, and then in pylifecycle.c an extern 
function can free up deep-frozen modules memory if Py_DEEPFROZEN_MODULES which 
will be defined in deepfreeze.c then it should work.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Dong-hee Na]

Change by Dong-hee Na :


--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Guido van Rossum]

Guido van Rossum  added the comment:

> Would it be possible to enhance deepfreeze be produce a list of all 
> (immortal) code objects?

That should be simple. We could see if Kumar is interested.

--
nosy: +kumaraditya303

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

Maybe there should be a way to traverse all immortal code objects at Python 
exit and clear their PyCodeObject.co_quickened member (and maybe some other 
members).

Would it be possible to enhance deepfreeze be produce a list of all (immortal) 
code objects?

I did something similar for static types with _PyStaticType_Dealloc() in 
bpo-46417, but it's easy since the list of static type is known in advance and 
it's short (100 to 200 types).

--

I'm working on fixing the very old bug bpo-1635741: Python must release all 
memory that it called in Py_Finalize(). It matters when Python is embedded in 
an application. It makes sense to call Py_Initialize()/Py_Finalize() multiple 
times in such use case. Python should not leak any memory.

With my PR 30815 fix + my msg411321 workaround, "./python -I -X showrefcount -c 
pass" now says that Python leaks exactly *zero* memory block: so bpo-1635741 is 
basically fixed, for the simplest Python command ("pass"). Obviously, I expect 
that more work is needed for more complex workload, since there are other 
static types which are still not cleared at Python exit, and more C extension 
modules which are not ported to the multi-phase initialization API (PEP 489).

Valgrind is just one way to see the issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[Guido van Rossum]

Guido van Rossum  added the comment:

If any of the immortal, deep-frozen code objects is ever quickened, I suppose 
the quickening data is never freed. But when we finalize and reinitialize, the 
co_quickened flag should remain set, so this would be a one-time leak.

The question is whether the quickening cache points to any objects that *are* 
freed. If it does, that could be bad. If it doesn't, then all we lose is a 
fixed amount of memory (no further leaks if we finalize and initialize the 
runtime repeatedly).

However, if my theory holds, why would valgrind consider the memory leaked? 
(TBH I don't know what valgrind does, so maybe that's not the right question.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

STINNER Victor  added the comment:

Patch to disable _Py_Quicken(), to help me debugging other memory leaks at 
Python exit:

diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
index dfc75300315..f9cdefed2a2 100644
--- a/Include/internal/pycore_code.h
+++ b/Include/internal/pycore_code.h
@@ -146,12 +146,14 @@ int _Py_Quicken(PyCodeObject *code);
 static inline int
 _Py_IncrementCountAndMaybeQuicken(PyCodeObject *code)
 {
+#if 0
 if (code->co_warmup != 0) {
 code->co_warmup++;
 if (code->co_warmup == 0) {
 return _Py_Quicken(code) ? -1 : 1;
 }
 }
+#endif
 return 0;
 }

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46476] Not all memory allocated by _Py_Quicken() is released at Python exit

[STINNER Victor]

Change by STINNER Victor :


--
title: Not all memory allocated by _Py_Quicken() is not released at Python exit 
-> Not all memory allocated by _Py_Quicken() is released at Python exit

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com