Opened 5 years ago

Last modified 3 years ago

#3194 open defect

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

Reported by: andreyv Owned by: er.anshul.maheshwari@gmail.com
Priority: minor Component: avcodec
Version: git-master Keywords: leak
Cc: barletz@gmail.com, er.anshul.maheshwari@gmail.com 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 andreyv 5 years ago.
file for test, only decoder initialization
valgrind_output.txt (6.3 KB) - added by andreyv 5 years ago.
valgrind output

Download all attachments as: .zip

Change History (22)

comment:1 Changed 5 years ago by cehoyos

  • Keywords leak added

Is the leak reproducible with current FFmpeg git head?

comment:2 Changed 5 years ago by andreyv

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

Changed 5 years ago by andreyv

file for test, only decoder initialization

Changed 5 years ago by andreyv

valgrind output

comment:3 Changed 5 years ago by cehoyos

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 Changed 5 years ago by cehoyos

  • Version changed from 2.1.1 to git-master

comment:5 Changed 5 years ago by andreyv

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 Changed 5 years ago by andreyv

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

comment:7 Changed 5 years ago by andreyv

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 Changed 5 years ago by cehoyos

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

comment:9 Changed 5 years ago by andreyv

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 Changed 5 years ago by andreyv

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 Changed 5 years ago by tomerb

  • 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 Changed 5 years ago by tomerb

  • Cc barletz@gmail.com added
  • Reproduced by developer set
  • Status changed from new to open

comment:13 Changed 5 years ago by er.anshul.maheshwari@gmail.com

  • Owner set to er.anshul.maheshwari@gmail.com

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 Changed 5 years ago by er.anshul.maheshwari@gmail.com

  • Cc er.anshul.maheshwari@gmail.com added

comment:15 Changed 5 years ago by michael

  • Priority changed from normal to minor
  • Summary changed from valgrind leak on example package to valgrind 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 Changed 3 years ago by jyavenard

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 Changed 3 years ago by jyavenard

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 Changed 3 years ago by pracucci

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 Changed 3 years ago by heleppkes

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 Changed 3 years ago by gjdfgh

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.)

Note: See TracTickets for help on using tickets.