Memory leak with destroy
Closed this issue · 36 comments
Important Information
Once create, initialize and LoadFile are used, memory leakage will occur in destroy.
Mpv version and platform
libmpv: mpv-dev-i686-20220206-git-0197729
system: window 10
ide: vs2019
Code segment
mpv_handle* mpv = nullptr;
void play() {
stop();
mpv = mpv_create();
mpv_initialize(mpv);
const char* args[] = { "loadfile", "1.mp4", NULL };
mpv_command_async(mpv, 0, args);
}
void stop() {
if(mpv) {
mpv_terminate_destroy(mpv);
}
}
Reproduction steps
Call the function play().
Expected behavior
No more allocates memory.
Actual behavior
Every use play function will allocates more 2-3M memory until it crashes.
Not sure what you have done to build mpv with MSVC, nice to know that it's apparently possible, but, as far as I know, it's not officially supported.
@cipher1985 Did you use https://github.com/jhuix/mpv-vsbuild?
Or what? Also please use normal VS 2022 compiler.
the libmpv use precompiler
https://sourceforge.net/projects/mpv-player-windows/files/libmpv/
mpv-dev-i686-20220213-git-bc9805c.7z
I use the new libmpv download with this url :
https://sourceforge.net/projects/mpv-player-windows/files/libmpv/?css-reload=2
the precompiler ver：
mpv-dev-i686-20220612-git-602995f.7z
but the memory also grow up.
Which wrong with the code?
ide: vs2019
system: win10
whole code:
#include <client.h>
#pragma comment(lib,"libmpv.dll.a")
mpv_handle* mpv = nullptr;
void stop() {
if (mpv) {
mpv_terminate_destroy(mpv);
}
}
void play() {
stop();
mpv = mpv_create();
mpv_initialize(mpv);
const char* args[] = { "loadfile", "1.mp4", NULL };
mpv_command_async(mpv, 0, args);
}
int main()
{
for (int i = 0; i < 1000; ++i) {
play(); //Why this function everytime make the memory grow up 2-3M?
Sleep(2000);
}
stop();
return 0;
}
i was able to narrow down the problem
without playing anything a loop like this:
mpvhandle = mpv_create()
mpv_initialize(mpvhandle)
Sleep 10
mpv_destroy(mpvhandle)
cause approx 1 MB of memory leak at every cycle on win32
removing mpv_initialize there is no leak...so definitvely libmpv is not unitializing something at destroy
can someone test it and confirm it?
the difference between memory allocated by mpv_initialize and freed by mpv_destroy is 768K
after 1000 cycles there is a leak of 768 MB
please fix it
I tested all available versions of libmpv.
the memory leak started on April 12, 2020 with this version:
https://master.dl.sourceforge.net/project/mpv-player-windows/libmpv/mpv-dev-i686-20200412-git-530a086.7z?viasf=1
mpv-version=mpv 0.32.0-352-g530a0863b8
ffmpeg-version=git-2020-04-12-f1894c206
the previous version has no memory leak:
mpv-version=mpv 0.32.0-328-gc5f8ec76b1
ffmpeg-version=git-2020-04-05-8b1f07ef5
now it's possible to compare the code changes in mpv_initialize and mpv_destroy between the 2 versions?
i woud really use the latest version and fix this massive leak
Actually, for me, this code just deadlocks on the second mpv_initialize call...
Edit: nvm, bug in my test code
I cannot reproduce this memory leak. On my end, memory usage stays more or less constant (going up and down by a few MB in each cycle, but not growing endlessly):
#include <mpv/client.h>
#include <stdio.h>
#include <unistd.h>
int main()
{
for (int i = 0; i < 1000; i++) {
printf("cycle %d\n", i);
mpv_handle *mpv = mpv_create();
mpv_initialize(mpv);
usleep(10000);
mpv_destroy(mpv);
}
pause();
}The final memory usage is 43 MB, same as before the first test. Test mpv version was mpv 0.35+git.20221220.d5c3b9d9
thank you for triyng this. what is your os version?
did you tried the 32 bits dll? or the 64bit?
this is my minimalistic code to reproduce the problem:
Declare LIB "mpv-2.dll"
Declare Function mpv_create CDecl () As Int
Declare Function mpv_initialize CDecl (ByVal mpv_handle As Int) As Int
Declare Sub mpv_destroy CDecl (ByVal mpv_handle As Int)
Dim mpvhandle As Int, counter As Int
For counter = 1 To 1000
mpvhandle = mpv_create()
mpv_initialize(mpvhandle)
Pause 1
mpv_destroy(mpvhandle)
Next counter
this is the result after 1000 loop
Note that whith this same code if i only replace mpv-2.dll (32 bits) with mpv-1.dll (32 bits) from 5 april 2020 or previous the memory stay quiet no memory leak
i attach a compiled sample of the code above (i have digitally signed it so i I certify that it is not a malicious executable)
can someone run it (put mpv-2.dll 32 bits near the exe) and tell me if the problem occurs?
Thank you
MPVTestLeak.zip
without playing anything a loop like this:
mpvhandle = mpv_create() mpv_initialize(mpvhandle) Sleep 10 mpv_destroy(mpvhandle)
cause approx 1 MB of memory leak at every cycle on win32
Can you run your program after setting the env var MPV_LEAK_REPORT=1 and post any reported leaks? preferably reduce the program to one iteration of create/destroy.
Can you run your program after setting the env var
MPV_LEAK_REPORT=1and post any reported leaks? preferably reduce the program to one iteration of create/destroy.
Unfortunately im unable to build libmpv binary by myself but if you give me a 32 bit debug version of libmpv that log memory allocations \ deallocations i will run it and show results.
I don't think you need the debug version. Just set the env var, then run the program which you already compiled with the (normal) dll which you downloaded, and report the result please.
pardon my ignorance, is MPV_LEAK_REPORT to be set as a an operating system environment variable?
and where do i expect to get the log? through the stdout? stderror?
You can set it like in your picture (and then you need to open a new console).
Or, simpler, if you open a cmd.exe console window, then first run this line:
set MPV_LEAK_REPORT=1
And then, from the same console, run your program by typing its name and enter (e.g. MPVTestLeak.Exe), and let it exit normally (don't abort it with ctrl-c).
If leaks are detected, then the output should be at the console. I don't recall if it's stdout or stderr, but I'm guessing stderr.
when i run my exe at the console it returns immeditely without waiting for the end execution (probably because my exe is not a consolle application but a standard desktop app). kindly you or haasn could you please compile for me a little exe that i will run here and post results? thank you.
I compiled this program:
// build:
// (mingw) gcc: gcc -o mpv-leak-test.gcc.exe mpv-leak-test.c -L. -lmpv
// (msvc 2015): cl mpv-leak-test.c mpv.dll.a
#include <windows.h>
#include <stdio.h>
//#include <unistd.h>
#include "include/client.h"
int main(int argc, char **argv)
{
int n = argc > 1 ? atoi(argv[1]) : 1;
for (int i = 0; i < n; i++) {
printf("%d ", i);
mpv_handle *mpv = mpv_create();
mpv_initialize(mpv);
Sleep(100);
mpv_destroy(mpv);
Sleep(100);
}
return 0;
}Using gcc first, then compiled using visual studio 2015 (64).
I tested it with mpv-2.dll (64) which I compiled myself.
The source, dll, and binaries (gcc 64, msvc 2015 64) are in this zip https://0x0.st/o5JR.zip .
EDIT: For reference, the dll at the zip is from 0.35.0-60-g874e28f4a4 (one commit behind current master 657fd28) plus some of my own patches on top, from this branch https://github.com/avih/mpv/commits/experiments-avih .
I let it iterate 500 times, and the program memory usage did not reach even 10M. Leaks were not reported either.
If you want to ensure that leaks are tested, delete the line mpv_destroy(mpv); and compile again, to see the leak report at the end. At the zip that's the file mpv-leak-test.msvc-2015.NO-DESTROY.exe.
I did not yet test with 32 bit dll downloaded from someone else. Maybe we can do that later.
thank you i want to test your stuff but the link https://0x0.st/o5JR.zip say
could you please send me another link?
thank you for triyng this. what is your os version?
did you tried the 32 bits dll? or the 64bit?
My earlier post has this info.
could you please send me another link?
No, I'm not going to try hosting sites until you can download. If you want, tell me where I can upload it so that you can download it.
this is my minimalistic code to reproduce the problem:
I'm not trying it. Let's stick to C.
Please use the program I tried (above, at the zip, and here below). Does it reproduce with it too?
// build:
// (mingw) gcc: gcc -o mpv-leak-test.gcc.exe mpv-leak-test.c -L. -lmpv
// (msvc 2015): cl mpv-leak-test.c mpv.dll.a
#include <windows.h>
#include <stdio.h>
//#include <unistd.h>
#include "include/client.h"
int main(int argc, char **argv)
{
int n = argc > 1 ? atoi(argv[1]) : 1;
for (int i = 0; i < n; i++) {
printf("%d ", i);
mpv_handle *mpv = mpv_create();
mpv_initialize(mpv);
Sleep(100);
mpv_destroy(mpv);
Sleep(100);
}
return 0;
}thank you i want to test your stuff but the link https://0x0.st/o5JR.zip say
could you please send me another link?
Sorry about this, 0x0.st’s virus scanner had a false positive on the libmpv build in there and removed it. Should be fixed.
could you please send me another link?
No, I'm not going to try hosting sites until you can download. If you want, tell me where I can upload it so that you can download it.
Sorry, the host thought it's a virus (because it has few executables compiled with two compilers?) and removed it, but now it's cleared as false-positive.
I've uploaded it again here https://0x0.st/o5JR.zip
ok i downloaded. i'll test and report it
i'm really confused now.
all your stuuf works fine...no memory leak ...but i tried this:
i replaced your mp-2.dll with the one downloaded from here: https://sourceforge.net/projects/mpv-player-windows/files/libmpv/mpv-dev-x86_64-20221218-git-f9d0b0c.7z/download
and it happened the same meory leak that ccurs in my app.
look:
how could be eplained?
can you try the same to you?
all your stuuf works fine...no memory leak ...but i tried this:
i replaced your mp-2.dll with the one downloaded from here: https://sourceforge.net/projects/mpv-player-windows/files/libmpv/mpv-dev-x86_64-20221218-git-f9d0b0c.7z/download
and it happened the same meory leak that ccurs in my app.
I can see the same if I run the same program and only replace the dll.
But I don't think this is a good test, because these are different DLLs, and you need to re-build the program again with the other DLL (and mpv.dll.a).
And when I did that, then the leaks go away, both when building with gcc, and when building with msvc.
So again, I can't reproduce your issue, also not with the DLL from sourceforge.
Can you please build my program, like the comment says, and check for leaks?
unfortunately i don't have a c compiler.
Normally i expect to be able to use a library directly like let's say libvlc ... i have a binary and an an include with all the functions that i can call from my app. i'm using client.h and mp-2.dll. as far as i know this should be enough but it produce memory leak. The last non leaking libmpv for me was mpv 0.32.0-328-gc5f8ec76b1. You can say "libmpv is only for c compilers" but at least until that date was fine for me and currently i'm using libvlc without problems. i'm not using mpv.dll.a, coudl be this the cause of my problems?
unfortunately i don't have a c compiler.
Right, I just noticed it wasn't you who opened this issue.
Anyway, Using the C code/program I posted, I could not create a leak.
I don't know how to help you anymore.
maybe a last help: are you able to compile a mp-2.dll 32 bits for me?
are you able to compile a mp-2.dll 32 bits for me?
No, sorry, I don't have the required setup for this.
Declare Function mpv_create CDecl () As Int
Declare Function mpv_initialize CDecl (ByVal mpv_handle As Int) As Int
Declare Sub mpv_destroy CDecl (ByVal mpv_handle As Int)
This looks like visual basic, and i'm completely unfamiliar with VB, so this is a shot in the dark.
How did you come up with these declarations?
mpv_handle is a struct, and the return value of mpv_create (and the argument of mpv_initialize and mpv_destroy) is a pointer to this struct.
You declare it as int, and I'm guessing that it can't hold the pointer, and therefore you don't pass the correct pointer to mpv_destroy (and also mpv_initialize), which maybe make it not destroy anything, hence a leak.
Also, why are there both Function and Sub declarations? shouldn't all of these be of the same call-type?
mpv_handle it's a pointer to a struct so it's a 32 bit address that can be stored without problem in a 32 bit integer. Me i don't have to access directly that structure, I only have to pass that address not the structure istelf to the lib functions, and indeed all libpmv functions works perfectly. An address could be right or wrong, if it's wrong you only get an access violation not a leak.
Sub are calls without return value ...if you look at client.h there are calls that not return any value
I finally figured out where the problem is.
i managed to compile mpv-2.dll i686 in my windows computer using msys2 and there is no memory leak with my builded version.
i followed this page: https://github.com/mpv-player/mpv/blob/master/DOCS/compile-windows.md#native-compilation-with-msys2
The problem only occurs with shinchiro versions (when used as dynamic dll). I hope shinchiro will investigate the problem that occurs with his i686 dlls starting from 2020-04-05. Before that date shinchiro i686 mpv-1.dll has no leaks
so we can conclude that there is no memory leak in the destroy function of mpv
unfortunately I still have one last problem ... msys2 does not compile a single mpv-2.dll with all linked dependencies but I have to use dozens of other dlls. I hope to find a solution to this too.
The problem only occurs with shinchiro versions
It might be related to different runtime used, maybe ucrt vs msvcrt. Usually you want to build the dll and your own program with similar enough environments. But anyway, we don't really have enough evidence to suggest that create+destroy on its own is leaking.
unfortunately I still have one last problem ... msys2 does not compile a single mpv-2.dll with all linked dependencies
This is completely off topic. It's because msys2 does not provide static mingw libraries. The main method to link statically with all the libs is to first compile all the libs statically yourslf - like shinchiro does, or using MXE (which is similarly lengthy).
Anyway, your issue of create->destroy->leak is most probably off topic for this issue, where the leak is after playing a file - #9860 (comment) .
Please don't post about your issue here (create/destroy -> leak). If you're convinced that create+destroy leaks, please open another issue (or request to re-open an existing issue on this subject).
no i'm not off topic. I have the same issue of cipher1985. He do not post anymore here but i'm sure that if he remove the loadfile call it has the same 768 kb memory leak caused by the i686 version of shinchiro. (if you read the post he also is using that) I've already opened another issue at shinchiro but it was immediately closed because nobody believes me but it doesn't matter. However, know that it is not possible to embed the schichiro i686 mpv-2dll into a non-stop application due to this leak. Now I will study alternative methods to build mpv-2.dll i686
i'm sure that if he remove the loadfile call it has the same 768 kb memory leak caused by the i686 version of shinchiro
I'm not sure because I can't know. If they confirm then you have the same issue, but until it's confirmed, to me it looks like different issues.
Old one but from skimming the discussion it looks like whatever leak was occurring here didn't come from mpv so closing.