Skip to content
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

[Windows] Embedded Python Memory Leaks #96853 ( Corrected 98%), Need testing #98359

Open
AlexSoft73 opened this issue Oct 17, 2022 · 17 comments
Open
Labels
pending The issue will be closed if no feedback is provided topic-subinterpreters type-bug An unexpected behavior, bug, or error

Comments

@AlexSoft73
Copy link

Hi Python team,

I detected python memory leaks when using it embedded on windows platform, unfortunately the issue I opened was closed.

I decided to correct the issue (though it is not 100% corrected), python now returns most of the memory it takes when finished.

I need test to be done with the modifications I did, especially if it requires any modifications for Unix based systems:

pylifecycle.c --> call function ____Py_ArenaMemoryRelease
obmalloc.c --> Define function ____Py_ArenaMemoryRelease pluss the trackig of memory allocations and releases

Pelase, test it and let me know.

Thanks you all
@AlexSoft73

@AlexSoft73 AlexSoft73 added the type-feature A feature request or enhancement label Oct 17, 2022
@iritkatriel
Copy link
Member

@AlexSoft73 I don't understand. Which test needs to be done?

@iritkatriel iritkatriel added the pending The issue will be closed if no feedback is provided label Dec 1, 2023
@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 6, 2023 via email

@vstinner
Copy link
Member

vstinner commented Dec 6, 2023

I don't understand how to reproduce your issue. Which Python version did you test?

Python 3.12 now releases most memory in Py_Finalize(), but not all stdlib extensions have been updated yet.

@vstinner
Copy link
Member

vstinner commented Dec 6, 2023

detected python memory leaks when using it embedded on windows platform, unfortunately the issue I opened was closed.

Which issue? Do you have a link?

@vstinner
Copy link
Member

vstinner commented Dec 6, 2023

Ah, wait, there are 2 other issues:

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 6, 2023 via email

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 6, 2023 via email

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 6, 2023 via email

@neonene
Copy link
Contributor

neonene commented Dec 8, 2023

Repeating OP's example 2000 times:

#include <Python.h>
int main(int argc, char *argv[])
{
    for (int N = 0; N < 2000; N++) {
        printf("%d\n", N);
        Py_InitializeEx(0);
        Py_FinalizeEx();
    }
    return 0;
}

When running this code on an embedded x64 executable built with MSVC v.1936, the following 3 commits from the left seem to cause serious memory consumption (taken from the Task Manager):

N 3.12.0a1+
67807cf
3.12.0a7+
ea2c001
3.13.0a0+
15d4c9f
3.11.x
3.12.0a1
500 330MB 770MB 1000MB 4MB
1000 650MB 1500MB 2000MB 4MB
1500 970MB 2240MB 3000MB 4MB
2000 1290MB 3000MB 4000MB 4MB

I'm not sure if they affect each other.
The python.org official binaries (.lib/.dll) can also be used to confirm.

cc @ericsnowcurrently @eduardo-elizondo @markshannon

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 8, 2023 via email

@neonene
Copy link
Contributor

neonene commented Dec 9, 2023

Is this test run tuning Peyton as main application or embedded?

As embedded.

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 10, 2023 via email

@neonene
Copy link
Contributor

neonene commented Dec 10, 2023

Your example at least needs EmptyWorkingSet() Windows API like:

    hProcess = OpenProcess(
        PROCESS_QUERY_INFORMATION |
        PROCESS_VM_READ |
        PROCESS_SET_QUOTA,  // added
        ...
    );
    ...

    if (EmptyWorkingSet(hProcess)) {  // added
        if (GetProcessMemoryInfo(hProcess, ...)) {
            ...
        }
    }
    CloseHandle(hProcess);

And I repeated the sequence of the main function to see the risk of an out-of-memory.
Then Python 3.11.7 shows:
Then the executable embedded with Python 3.11.7 shows:

(1st)
Memory usage: 217088  // BeforePyInit
Memory usage: 212992  // AfterPyInit 
Memory usage: 204800  // AfterPyFinalized 
Memory usage difference between start and end: -12288
(2nd)
Memory usage: 200704
Memory usage: 212992
Memory usage: 196608
Memory usage difference between start and end: -4096
(3rd)
Memory usage: 196608
Memory usage: 212992
Memory usage: 204800
Memory usage difference between start and end: 8192
(4th)
Memory usage: 200704
Memory usage: 212992
Memory usage: 200704
Memory usage difference between start and end: 0
(5th)
Memory usage: 200704
Memory usage: 212992
Memory usage: 200704
Memory usage difference between start and end: 0
...

If there is any leak, EmptyWorkingSet() cannot reduce the usage constantly like that. Note that GetProcessMemoryInfo() is not recommended for checking a memory leak. See:

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 10, 2023 via email

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 12, 2023 via email

@neonene
Copy link
Contributor

neonene commented Dec 12, 2023

Yes. Any tiny leakage from any version of Python will be accumulated in a process if an executable repeats loading/freeing the dynamic DLL which is linked to Python.
Currently, embedding does not mean plugging in/out. 3.12 and newer are under reducing global objects, which has another issue to be opened.

@AlexSoft73
Copy link
Author

AlexSoft73 commented Dec 12, 2023 via email

@ericsnowcurrently ericsnowcurrently added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending The issue will be closed if no feedback is provided topic-subinterpreters type-bug An unexpected behavior, bug, or error
Projects
Status: Todo
Development

No branches or pull requests

5 participants