-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Memory leak on executables embedded with 3.13 #113055
Comments
That doesn't look so nice, I can investigate. How are you measuring memory usage and what's the OS? In the previous versions of Python the obmalloc radix tree data structures were in the BSS and so most of RAM used by them was actually virtual and not real. I'm not sure if the move to interpreter state structures lost that behaviour but it could be cause for an increase in (real) memory use. Maybe there is also a memory leak there, e.g. malloc without free. |
Only the Task Manager on Windows 10 and 11 for me. |
I spent some time looking into this today. I suspect the fix is not going to be simple, unfortunately. What was previously global variables in the obmalloc.c module has become part of the PyInterpreterState structure. When an interpreter is freed, memory allocated by obmalloc doesn't get freed. I think that's the leak you are seeing. Previous to A possible way to fix this: when If you free the arenas and there is allocated blocks inside of them, you can't call |
Building gh-113218 with the address sanitizer and then running |
I can also personally confirm that on Windows, including the 32-bit version of Python. |
I think I've come up with a better fix. See gh-113412. |
ea2c001 has a memory leak to be fixed, which is found at gh-113412. |
Your
Trace log: https://gist.github.com/nascheme/fac2dd566f09fe3d75c98134f18c6170 E.g.
And
|
The following change seems to greatly reduce the leaks. The
|
My memory leak seems to come from cpython/Objects/unicodeobject.c Lines 14952 to 14973 in 0c57454
|
For the record, my memory gain reduced from 2M bytes to 200K bytes per cycle with gh-113412 (non-debug). Regarding the crash after re-initialization due to the full cleanup by
A dangling pointer in a non-isolable (small) extension could be worked around by making the interned string statically allocated, if any. Original issue: #100911 |
My previous comment about disabling the
If Py_DEBUG is enabled, then that function will free interned strings with the flag |
Yes, I believe a good portion of the memory leaks are coming from not cleaning up interned strings. This was intentional given that during the time that gh-19474 was merged, there were PyObjects leaks that would be referenced in the next Py_Initialize. Therefore, we couldn't safely clean up these interned strings. Given that these PyObjects leaks have now been largely fixed, it should be safe to re-enable this code: gh-113601. |
Context: (expand)runtime state and lifecyle
embedders:
interpreter lifecycle:
consolidating global runtime state
interpreter isolation
immortal objects (PEP 683):
extension modulesglobal state and interpreter isolation:
single-phase init ("legacy" extensions):
multi-phase init:
challenges:
obmalloc state
With all that in mind, here's where I understand we're at now:
From there, something along the lines of what @nascheme has proposed seems reasonable. We also need to clean up interned strings as @eduardo-elizondo has proposed. It would be nice if we could be a bit more proactive about cleaning up single-phase init modules (at the least static types defined therein), but that might not be feasible. One other option: use a process-global allocator for immortal, immutable objects (e.g. |
Regarding interned strings cleanup, see also: |
For interpreters that share state with the main interpreter, this points to the same static memory structure. For interpreters with their own obmalloc state, it is heap allocated. Add free_obmalloc_arenas() which will free the obmalloc arenas and radix tree structures for interpreters with their own obmalloc state. Co-authored-by: Eric Snow <[email protected]>
) For interpreters that share state with the main interpreter, this points to the same static memory structure. For interpreters with their own obmalloc state, it is heap allocated. Add free_obmalloc_arenas() which will free the obmalloc arenas and radix tree structures for interpreters with their own obmalloc state. Co-authored-by: Eric Snow <[email protected]>
) For interpreters that share state with the main interpreter, this points to the same static memory structure. For interpreters with their own obmalloc state, it is heap allocated. Add free_obmalloc_arenas() which will free the obmalloc arenas and radix tree structures for interpreters with their own obmalloc state. Co-authored-by: Eric Snow <[email protected]>
GH-118618 would fix the most serious memory leak in 3.12. For the last bugfix release of 3.12, @Yhg1s decided it was too large/disruptive to merge a backport and so that PR is closed. Assuming we are not changing our minds on merging that, I think the memory leak due to the obmalloc state will remain unfixed in 3.12. In theory there might be a less invasive and simpler fix for 3.12, I'd have to research that though. It's getting confusing to keep track of these embedding bugs and what fixes have been backported. There are two classes of them: memory leaks and crashers. The crashers are the more serious ones. I believe we have fixes the known crashers (see GH-124865, GH-124536, GH-125314). For the memory leaks, the obmalloc state one is the worst. The leaks due to interned strings also depend on the mortal string change (49f6beb). Running the 3.12 head with ASAN enabled would show what's still leaking. In the long term, i think it would be nice to have CPython running with no ASAN warnings. Then we have a build-bot to ensure we don't regress on that. |
FYI, 3.12.7 was not the last bugfix release of 3.12. We have scheduled bugfix releases of 3.12 until April 2025. |
I cannot reproduce this issue anymore on the main branch: diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index d15dd519dbf..3f9455a197f 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -38,7 +38,7 @@ char **main_argv;
#define PROGRAM_NAME L"./_testembed"
#define PROGRAM_NAME_UTF8 "./_testembed"
-#define INIT_LOOPS 4
+#define INIT_LOOPS 100
// Ignore Py_DEPRECATED() compiler warnings: deprecated functions are
// tested on purpose here.
@@ -220,6 +220,10 @@ static int test_repeated_simple_init(void)
_testembed_Py_Initialize();
Py_Finalize();
printf("Finalized\n"); // Give test_embed some output to check
+
+ char cmd[1000];
+ sprintf(cmd, "grep ^VmRSS /proc/%i/status", getpid());
+ system(cmd);
}
return 0;
} Output:
Since 3.12 branch is not going to be fixed, can we close the issue? |
Bug report
Bug description:
Since 67807cf,
_testembed.exe
(x64 Release) increases the memory usage in the cycle ofPy_Initialize
andPy_Finalize
.I monitored the Task Manager (
taskmgr.exe
) on Windows10/11, with a change againstPrograms/_testembed.c
:Commands:
_testembed.exe test_repeated_simple_init
orpython -m test test_embed -m test_simple_initialization_api
(no log)With RADIX TREE (as-is)
No RADIX TREE (as-is)
Recent
obmalloc.c
looks ready for finalization. Just adding my rough (invalid?) experiment made the leaks even. So, I hope that they share the same issue. Otherwise, ea2c001 or 15d4c9f has another leak. #98359 (comment)patch
With RADIX TREE (patched)
No RADIX TREE (patched)
cc @ericsnowcurrently @nascheme
CPython versions tested on:
3.12, 3.13, CPython main branch: 3aea6c4
Operating systems tested on:
Windows
Linked PRs
The text was updated successfully, but these errors were encountered: