Opened 9 years ago

Closed 15 months ago

#3194 closed defect (fixed)

valgrind leak on example package (leaking global lock manager mutex at exit)

Reported by: Andrey Owned by: Anshul
Priority: minor Component: avcodec
Version: git-master Keywords: leak
Cc: barletz@gmail.com, Anshul Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: yes

Description

Hi

I've found valgrind leak when making and running valgrind on examples/decoding_encoding from latest 2.1 branch.
A small example is in doc/example , called decoding_encoding

Leak summary:

==22764== 56 bytes in 1 blocks are possibly lost in loss record 1 of 1
==22764== at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==22764== by 0x85753B0: av_malloc (in /usr/ffmpeg/build/LibFFMPEG/ffmpeg-2.1/doc/examples/decoding_encoding)
==22764== by 0x807A353: video_decode_example (decoding_encoding.c:107)
==22764== by 0x807A3FF: main (decoding_encoding.c:174)

Actually, I have the same issue in my big application. I switched from 0.8 branch to 2.1 branch, changed API and most of the files decoded fine. However, I am working on a valgrind leak summary always in the same place for all codecs (actually it is start of decoder allocation). Why does it happening ? What I am doing wrong ? Could you help me with this issue ?

How to reproduce:
make 2.1 branch with or anything else , I tried many different configurations

./configure --prefix="/usr/ffmpeg/build/LibFFMPEG" --extra-ldflags="-L/usr/ffmpeg/build/LibFFMPEG/lib" --bindir="/usr/ffmpeg/build/LibFFMPEG/bin" --enable-pthreads --extra-libs="-ldl" --extra-cflags="-I/usr/ffmpeg/build/LibFFMPEG/include" --enable-nonfree --enable-gpl --disable-vaapi --disable-debug --disable-iconv --disable-doc --disable-encoders --enable-memalign-hack'

  1. make examples from doc/examples

3 run valgrind --tool=memcheck --leak-check=full --show-reachable=yes --verbose --track-origins=yes ./decoding_encoding <example> <example>

Attachments (2)

decoding_encoding.c (20.1 KB ) - added by Andrey 9 years ago.
file for test, only decoder initialization
valgrind_output.txt (6.3 KB ) - added by Andrey 9 years ago.
valgrind output

Download all attachments as: .zip

Change History (23)

comment:1 by Carl Eugen Hoyos, 9 years ago

Keywords: leak added

Is the leak reproducible with current FFmpeg git head?

comment:2 by Andrey, 9 years ago

Hi

I've done the same on latest git
The configuration this time is
./configure --prefix="/home/andrey/ffmpeg_sources/ffmpeg_latest" --extra-ldflags="-L/home/andrey/ffmpeg_build/lib" --bindir="/home/andrey/ffmpeg_build/bin" --enable-pthreads --extra-libs="-ldl" --extra-cflags="-I/home/andrey/ffmpeg_build/include" --enable-nonfree --enable-gpl --disable-vaapi --disable-iconv --enable-memalign-hack

I've commented everything in decode_encode.c and attaching the copy of changed simple example
I really do not require to see the problems in decoder or encoder. My problem is a leak on initialization stage. The previous branch my project was linked 0.8 has no this issue
I am also ataching the valgrind output for you

by Andrey, 9 years ago

Attachment: decoding_encoding.c added

file for test, only decoder initialization

by Andrey, 9 years ago

Attachment: valgrind_output.txt added

valgrind output

comment:3 by Carl Eugen Hoyos, 9 years ago

Why do you use --enable-memalign-hack ?
Please also remove --enable-pthreads --enable-nonfree from your configure line, both have no effect.

The relevant part of your valgrind output is:

==19116== 56 bytes in 1 blocks are possibly lost in loss record 1 of 1
==19116==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==19116==    by 0x86816B0: av_malloc (mem.c:86)
==19116==    by 0x808CB82: video_decode_example (decoding_encoding.c:557)
==19116==    by 0x808CC9B: main (decoding_encoding.c:635)

Line 557 of decoding_encoding.c does not contain a call to av_malloc() or do I miss something?

comment:4 by Carl Eugen Hoyos, 9 years ago

Version: 2.1.1git-master

comment:5 by Andrey, 9 years ago

It is no av_malloc there. It should be called from avcodec_open2.
At least it seems to be.
The leak in av_malloc as it assign more than enough maybe, that's not freed at the end
I've not investigated from source code perspective, only from library usage

comment:6 by Andrey, 9 years ago

Actually I passed my win32 project in Dr. memory and the leak exist there also only on init stage from avcodec_open2

comment:7 by Andrey, 9 years ago

I've tested with suggested arguments. However , it doesn't help to fix the leak

./configure --prefix=/home/andrey/ffmpeg_sources/ffmpeg_latest --extra-ldflags=-L/home/andrey/ffmpeg_build/lib --bindir=/home/andrey/ffmpeg_build/bin --extra-libs=-ldl --extra-cflags=-I/home/andrey/ffmpeg_build/include --enable-gpl --disable-vaapi --disable-iconv

==6788== 24 bytes in 1 blocks are still reachable in loss record 1 of 1
==6788== at 0x402A420: memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==6788== by 0x402A4DE: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==6788== by 0x86816A7: av_malloc (mem.c:94)
==6788== by 0x808CB22: video_decode_example (decoding_encoding.c:557)
==6788== by 0x808CC3B: main (decoding_encoding.c:635)

line 557 is

if (avcodec_open2(c, codec, NULL) < 0) {

fprintf(stderr, "Could not open codec\n");
exit(1);

}

the leak comes from the av_malloc, it is inside the allocation from avcodec_open2

comment:8 by Carl Eugen Hoyos, 9 years ago

Please build with --disable-optimizations this should make the valgrind output more useful.

comment:9 by Andrey, 9 years ago

ok now the output:
==15887== 24 bytes in 1 blocks are still reachable in loss record 1 of 1
==15887== at 0x402A420: memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==15887== by 0x402A4DE: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==15887== by 0x874A4BC: av_malloc (mem.c:94)
==15887== by 0x84AFD9B: default_lockmgr_cb (utils.c:78)
==15887== by 0x84B867A: ff_lock_avcodec (utils.c:3240)
==15887== by 0x84B2500: avcodec_open2 (utils.c:1180)
==15887== by 0x8049ED2: video_decode_example (decoding_encoding.c:557)
==15887== by 0x8049FEB: main (decoding_encoding.c:635)

comment:10 by Andrey, 9 years ago

I can't see why those functions are leaking
==15887== by 0x84AFD9B: default_lockmgr_cb (utils.c:78)
==15887== by 0x84B867A: ff_lock_avcodec (utils.c:3240)
==15887== by 0x84B2500: avcodec_open2 (utils.c:1180)

The leak is small 26bytes. However, if you open 1000 decoders or 10000 it converts to be not so small

comment:11 by Tomer Barletz, 9 years ago

Analyzed by developer: set

The leak is caused by malloc(ing) a mutex in default_lockmgr_cb() upon first use, and never free(int) it.

comment:12 by Tomer Barletz, 9 years ago

Cc: barletz@gmail.com added
Reproduced by developer: set
Status: newopen

comment:13 by Anshul, 8 years ago

Owner: set to Anshul

Reproduced with git master,
Given a patch for new api to destroy mutex.
But is not very good patch, user will we required to call ff_destroy_lock_avcodec function.

User who are using some older version of ffmpeg can get rid of this leak by passing there own
lock, since problem is in default lock of avcodec.

comment:14 by Anshul, 8 years ago

Cc: Anshul added

comment:15 by Michael Niedermayer, 8 years ago

Priority: normalminor
Summary: valgrind leak on example packagevalgrind leak on example package (leaking global lock manager mutex at exit)

this leak is just a single mutex for the whole program execution and will get freed by the OS after program execution. So the only issue that this causes is a warning in tools like valgrind.
Also its non trivial to free this mutex without race conditions

comment:16 by jyavenard, 6 years ago

Why wasn't the codec_mutex not allocated on the stack instead ?

The test that the codec_mutex has been allocated isn't thread-safe itself.
(testing that a pointer isn't null isn't an atomic operation, and the behaviour is undefined in C or C++)

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/utils.c#L76

        if (!*mutex) {
            pthread_mutex_t *tmp = av_malloc(sizeof(pthread_mutex_t));
            if (!tmp)
                return AVERROR(ENOMEM);

so I don't see much the advantage in allocating codec_mutex on the stack ; and would get around valgrind reporting this has a leak.

(I'm facing the same problem on our mozilla's ASAN build which has suddenly started to report failures to this 40 bytes (size of pthread_mutex_t on Linux 64 bits) object leaking)

comment:17 by jyavenard, 6 years ago

calling av_lockmgr_register(nullptr) solved the problem, it will de-allocate the mutexes

This API exists on pretty much all version of libavcodec, why not use that instead?

nicer than a new ff_destroy_lock_avcodec function.

comment:18 by Marco Pracucci, 6 years ago

I'm testing ffmpeg 3.0.2 and I can still reproduce this memory leak. Is there any news on the fix?

Thank you

comment:19 by Hendrik, 6 years ago

examples often skip a bit of cleanup and error checking for brevity, if your application suffers from such a problem, its your responsibility to clear it out.

comment:20 by gjdfgh, 6 years ago

This is a harmless leak, and it will prevent you from running into really obscure race conditions. I suggest closing as WONTFIX.

(Although we could implement statically initializable mutexes with SRWs on Windows.)

comment:21 by mkver, 15 months ago

Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.