#4819 closed defect (invalid)
av_register_all() memory leak
| Reported by: | joeallen | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | avcodec |
| Version: | unspecified | Keywords: | libx265 leak |
| Cc: | Blocked By: | ||
| Blocking: | Reproduced by developer: | no | |
| Analyzed by developer: | no |
Description
Originally I was making an application to convert audio to wav, and then I want to make sure it didn't leak, so I ran it with valgrind. There were a bunch of leaks but apparently av_register_all() is leaking memory. To confirm, I made a small test program:
#include <math.h>
#include <string>
#include <queue>
#include <deque>
#include <iostream>
//#ifdef __cplusplus
extern "C" {
#ifndef __STDC_CONSTANT_MACROS
# define __STDC_CONSTANT_MACROS
#endif
#include <libavformat/avformat.h>
#include <libavcodec/avcodec.h>
#include <libswresample/swresample.h>
#include <libswscale/swscale.h>
#include <libavutil/channel_layout.h>
#include <libavutil/common.h>
#include <libavutil/imgutils.h>
#include <libavutil/mathematics.h>
#include <libavutil/avassert.h>
#include <libavutil/avstring.h>
#include <libavutil/frame.h>
#include <libavutil/opt.h>
#include <libavutil/samplefmt.h>
#include <libavutil/timestamp.h>
#include <libavfilter/avfilter.h>
#include <libavfilter/buffersrc.h>
#include <libavfilter/buffersink.h>
#include <libavutil/dict.h>
}
int main (int argc, char ** argv){
//Initialize all codecs
av_register_all();
}
Here is the valgrind output.
valgrind --leak-check=full --show-leak-kinds=all ./mfpeg ==2417== Memcheck, a memory error detector ==2417== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==2417== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info ==2417== Command: ./mfpeg ==2417== ==2417== ==2417== HEAP SUMMARY: ==2417== in use at exit: 81 bytes in 2 blocks ==2417== total heap usage: 4 allocs, 2 frees, 167 bytes allocated ==2417== ==2417== 32 bytes in 1 blocks are still reachable in loss record 1 of 2 ==2417== at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2417== by 0x6DE768F: _dlerror_run (dlerror.c:141) ==2417== by 0x6DE70C0: dlopen@@GLIBC_2.2.5 (dlopen.c:87) ==2417== by 0x8DC7273: x265_api_get_63 (in /usr/local/lib/libx265.so.63) ==2417== by 0x5958E74: ??? (in /usr/local/lib/libavcodec.so.56.41.100) ==2417== by 0x59D76B1: avcodec_register_all (in /usr/local/lib/libavcodec.so.56.41.100) ==2417== by 0x4E6B342: av_register_all (in /usr/local/lib/libavformat.so.56.36.100) ==2417== by 0x400830: main (main.cpp:35) ==2417== ==2417== 49 bytes in 1 blocks are still reachable in loss record 2 of 2 ==2417== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2417== by 0x400FDC0: _dl_signal_error (dl-error.c:90) ==2417== by 0x40146CF: _dl_open (dl-open.c:715) ==2417== by 0x6DE702A: dlopen_doit (dlopen.c:66) ==2417== by 0x400FFF3: _dl_catch_error (dl-error.c:187) ==2417== by 0x6DE762C: _dlerror_run (dlerror.c:163) ==2417== by 0x6DE70C0: dlopen@@GLIBC_2.2.5 (dlopen.c:87) ==2417== by 0x8DC7273: x265_api_get_63 (in /usr/local/lib/libx265.so.63) ==2417== by 0x5958E74: ??? (in /usr/local/lib/libavcodec.so.56.41.100) ==2417== by 0x59D76B1: avcodec_register_all (in /usr/local/lib/libavcodec.so.56.41.100) ==2417== by 0x4E6B342: av_register_all (in /usr/local/lib/libavformat.so.56.36.100) ==2417== by 0x400830: main (main.cpp:35) ==2417== ==2417== LEAK SUMMARY: ==2417== definitely lost: 0 bytes in 0 blocks ==2417== indirectly lost: 0 bytes in 0 blocks ==2417== possibly lost: 0 bytes in 0 blocks ==2417== still reachable: 81 bytes in 2 blocks ==2417== suppressed: 0 bytes in 0 blocks ==2417== ==2417== For counts of detected and suppressed errors, rerun with: -v ==2417== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
I made a stackoverflow post on it: http://stackoverflow.com/questions/32323093/ffmpeg-av-register-all-memory-leak
It seems to be an issue with x265 codec. Any help would be appreciated. Thanks.
Change History (14)
comment:1 by , 9 years ago
| Keywords: | libx265 leak added |
|---|
comment:2 by , 9 years ago
It's not in ffmpeg, I think. I'm not even sure it's in libx265.
This is probably related: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=700899
comment:3 by , 9 years ago
I am using ffmpeg 2.7.1. I am gonna install 2.7.2 and see if it makes any difference. In the changelog there was this:
2015-06-27 Derek Buitenhuis configure: Check for x265_api_get
Gut feeling is that it won't do much I can see what happens.
comment:4 by , 9 years ago
(Not really sure why you'd care about something so minor, av_register_all() is a broken concept anyway.)
comment:5 by , 9 years ago
@gjdfgh
Good programming practices I and raise awareness..i guess. What did you mean by "av_register_all() is a broken concept?"
comment:6 by , 9 years ago
What did you mean by "av_register_all() is a broken concept?"
In a perfect world, this API call wouldn't exist. Because it's useless and changes global state.
But as it is, you have to call it once, and if it leaks 100 bytes, so what.
(Though the underlying issue is just with dlopen(), which it not directly related, but maybe unavoidable as well.)
comment:7 by , 9 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
If I understand Alessandro Ghedini's analysis correctly, this is a false positive.
comment:8 by , 9 years ago
| Resolution: | invalid |
|---|---|
| Status: | closed → reopened |
So, here's what I take home from it. In AVCodec, we have an init_static_data callback that can allocate, well, static data, and this is called for each codec that implements it during avregister_all. We have no way of free'ing such data: we don't have avunregister_all, and we don't have a deinit_static_data callback in AVCodec. We need both of these, and that's what this ticket is for.
Additionally, for this specific issue, libx265.c uses this init_static_data callback to call x265_api_get, which calls dlopen and leaks the local variable h (https://bitbucket.org/multicoreware/x265/src/b1af4c36f48a4500a4912373ebcda9a5540b5c15/source/encoder/api.cpp?at=default line 332). The API docs claim that we should call x265_cleanup() to clean up such data, but h is still lost. This is a x265 bug: it should not leak the local variable h, instead it should dlclose() it in x265_cleanup.
comment:9 by , 9 years ago
While I cannot reproduce an increasing leak no matter how often I call x265_api_get(), the leak is significantly bigger if the libraries for different bit-depths are actually available:
==26924== HEAP SUMMARY: ==26924== in use at exit: 3,244 bytes in 9 blocks ==26924== total heap usage: 11 allocs, 2 frees, 3,548 bytes allocated ==26924== ==26924== 30 bytes in 1 blocks are still reachable in loss record 1 of 7 ==26924== at 0x4C2ABED: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x4005D9D: open_path (in /lib64/ld-2.15.so) ==26924== by 0x4008266: _dl_map_object (in /lib64/ld-2.15.so) ==26924== by 0x401281D: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== by 0x4007B1: main (in a.out) ==26924== ==26924== 30 bytes in 1 blocks are still reachable in loss record 2 of 7 ==26924== at 0x4C2ABED: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x4005D9D: open_path (in /lib64/ld-2.15.so) ==26924== by 0x4008266: _dl_map_object (in /lib64/ld-2.15.so) ==26924== by 0x401281D: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== by 0x4007CD: main (in a.out) ==26924== ==26924== 32 bytes in 1 blocks are still reachable in loss record 3 of 7 ==26924== at 0x4C292B8: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x5C0E65F: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== by 0x4007B1: main (in a.out) ==26924== ==26924== 240 bytes in 1 blocks are still reachable in loss record 4 of 7 ==26924== at 0x4C292B8: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x4010042: _dl_check_map_versions (in /lib64/ld-2.15.so) ==26924== by 0x4012D05: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== by 0x4007B1: main (in a.out) ==26924== ==26924== 240 bytes in 1 blocks are still reachable in loss record 5 of 7 ==26924== at 0x4C292B8: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x4010042: _dl_check_map_versions (in /lib64/ld-2.15.so) ==26924== by 0x4012D05: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== by 0x4007CD: main (in a.out) ==26924== ==26924== 316 bytes in 2 blocks are still reachable in loss record 6 of 7 ==26924== at 0x4C2ABED: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x4C2AD6F: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x400AF71: _dl_new_object (in /lib64/ld-2.15.so) ==26924== by 0x40062D5: _dl_map_object_from_fd (in /lib64/ld-2.15.so) ==26924== by 0x4008152: _dl_map_object (in /lib64/ld-2.15.so) ==26924== by 0x401281D: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== ==26924== 2,356 bytes in 2 blocks are still reachable in loss record 7 of 7 ==26924== at 0x4C292B8: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26924== by 0x400ADEE: _dl_new_object (in /lib64/ld-2.15.so) ==26924== by 0x40062D5: _dl_map_object_from_fd (in /lib64/ld-2.15.so) ==26924== by 0x4008152: _dl_map_object (in /lib64/ld-2.15.so) ==26924== by 0x401281D: dl_open_worker (in /lib64/ld-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x401228B: _dl_open (in /lib64/ld-2.15.so) ==26924== by 0x5C0E015: dlopen_doit (in /lib64/libdl-2.15.so) ==26924== by 0x400E5F5: _dl_catch_error (in /lib64/ld-2.15.so) ==26924== by 0x5C0E5EB: _dlerror_run (in /lib64/libdl-2.15.so) ==26924== by 0x5C0E0B0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.15.so) ==26924== by 0x4ED78EA: x265_api_get_74 (in libx265.so.74) ==26924== ==26924== LEAK SUMMARY: ==26924== definitely lost: 0 bytes in 0 blocks ==26924== indirectly lost: 0 bytes in 0 blocks ==26924== possibly lost: 0 bytes in 0 blocks ==26924== still reachable: 3,244 bytes in 9 blocks ==26924== suppressed: 0 bytes in 0 blocks
#include "x265.h"
int main()
{
printf("x265_api_get( 0): %p \n", x265_api_get(0));
printf("x265_api_get( 8): %p \n", x265_api_get(8));
printf("x265_api_get(10): %p \n", x265_api_get(10));
printf("x265_api_get(12): %p \n", x265_api_get(12));
x265_cleanup();
x265_api_get(0);
x265_api_get(8);
x265_api_get(10);
x265_api_get(12);
x265_cleanup();
return 0;
}
follow-ups: 11 12 comment:10 by , 9 years ago
Also just to throw this out there: avfilter_register_all() also leaks additional 49 bytes. Eventually I am gonna have multiple instances(1K+) of this conversion application that I am making so those bytes would add up.
int main (int argc, char ** argv){
//Initialize all codecs
av_register_all();
avfilter_register_all();
}
Valgrind output:
HEAP SUMMARY: ==6024== in use at exit: 130 bytes in 3 blocks ==6024== total heap usage: 1,383 allocs, 1,380 frees, 573,201,253 bytes allocated ==6024== ==6024== Searching for pointers to 3 not-freed blocks ==6024== Checked 23,000,312 bytes ==6024== ==6024== 32 bytes in 1 blocks are still reachable in loss record 1 of 3 ==6024== at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==6024== by 0x714A68F: _dlerror_run (dlerror.c:141) ==6024== by 0x714A197: dlsym (dlsym.c:70) ==6024== by 0x110466BD: ??? (in /usr/lib/libGL.so.340.76) ==6024== by 0x1102A515: ??? (in /usr/lib/libGL.so.340.76) ==6024== by 0x40100FC: call_init.part.0 (dl-init.c:64) ==6024== by 0x4010222: _dl_init (dl-init.c:36) ==6024== by 0x4001309: ??? (in /lib/x86_64-linux-gnu/ld-2.19.so) ==6024== by 0x1: ??? ==6024== by 0xFFF00008A: ??? ==6024== by 0xFFF000092: ??? ==6024== ==6024== 49 bytes in 1 blocks are still reachable in loss record 2 of 3 ==6024== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==6024== by 0x5B70839: strdup (strdup.c:42) ==6024== by 0x1104891E: ??? (in /usr/lib/libGL.so.340.76) ==6024== by 0x1102A703: ??? (in /usr/lib/libGL.so.340.76) ==6024== by 0x40100FC: call_init.part.0 (dl-init.c:64) ==6024== by 0x4010222: _dl_init (dl-init.c:36) ==6024== by 0x4001309: ??? (in /lib/x86_64-linux-gnu/ld-2.19.so) ==6024== by 0x1: ??? ==6024== by 0xFFF00008A: ??? ==6024== by 0xFFF000092: ??? ==6024== ==6024== 49 bytes in 1 blocks are still reachable in loss record 3 of 3 ==6024== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==6024== by 0x400FDC0: _dl_signal_error (dl-error.c:90) ==6024== by 0x40146CF: _dl_open (dl-open.c:715) ==6024== by 0x714A02A: dlopen_doit (dlopen.c:66) ==6024== by 0x400FFF3: _dl_catch_error (dl-error.c:187) ==6024== by 0x714A62C: _dlerror_run (dlerror.c:163) ==6024== by 0x714A0C0: dlopen@@GLIBC_2.2.5 (dlopen.c:87) ==6024== by 0xD7E9273: x265_api_get_63 (in /usr/local/lib/libx265.so.63) ==6024== by 0x5F21E9C: ??? (in /usr/local/lib/libavcodec.so.56.41.100) ==6024== by 0x5FA0C21: avcodec_register_all (in /usr/local/lib/libavcodec.so.56.41.100) ==6024== by 0x4E6B452: av_register_all (in /usr/local/lib/libavformat.so.56.36.100) ==6024== by 0x4009D0: main (main.cpp:38) ==6024== ==6024== LEAK SUMMARY: ==6024== definitely lost: 0 bytes in 0 blocks ==6024== indirectly lost: 0 bytes in 0 blocks ==6024== possibly lost: 0 bytes in 0 blocks ==6024== still reachable: 130 bytes in 3 blocks ==6024== suppressed: 0 bytes in 0 blocks ==6024== ==6024== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) ==6024== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
comment:11 by , 9 years ago
Replying to joeallen:
Also just to throw this out there: avfilter_register_all() also leaks additional 49 bytes.
And how should this be fixed?
On my system there is no way to free resources allocated by dlopen() (at least from valgrind pov).
And please use a debug build for valgrind, the output you posted is not very helpful.
follow-up: 13 comment:12 by , 9 years ago
Replying to joeallen:
Also just to throw this out there: avfilter_register_all() also leaks additional 49 bytes. Eventually I am gonna have multiple instances(1K+) of this conversion application that I am making so those bytes would add up.
==6024== still reachable: 130 bytes in 3 blocks
Do you realize that “still reachable” means that the memory is not leaked, it is definitely allocated, because it is needed? Do you also realize that modern operating systems free all allocated memory when a process exit?
Therefore, freeing these 130 octets will only make a practical difference if your “1K+” instances all finish in the same millisecond and another very expensive application starts requiring memory in the small interval before they exit. Not very likely, to say the least.
Furthermore, since memory allocation is made by 4k pages at the process level, freeing 130 octets will likely make no difference at all.
Last of all, these 130 octets are needed during the works of the library. If the library were changed to allow freeing them, they still would be needed when it is in use. Actually, avoiding global state would likely increase the memory use marginally, due to the use of an extra indirection.
In short: you are barking at the wrong tree.
Note that I am all in favour of removing all global allocated state. But not due to valgrind's output, it is currently satisfactory. The good reason would be API cleanliness, and it would warrant the small extra memory requirement.
comment:13 by , 9 years ago
Replying to Cigaes:
Replying to joeallen:
Also just to throw this out there: avfilter_register_all() also leaks additional 49 bytes. Eventually I am gonna have multiple instances(1K+) of this conversion application that I am making so those bytes would add up.
==6024== still reachable: 130 bytes in 3 blocksDo you realize that “still reachable” means that the memory is not leaked, it is definitely allocated, because it is needed? Do you also realize that modern operating systems free all allocated memory when a process exit?
Therefore, freeing these 130 octets will only make a practical difference if your “1K+” instances all finish in the same millisecond and another very expensive application starts requiring memory in the small interval before they exit. Not very likely, to say the least.
Furthermore, since memory allocation is made by 4k pages at the process level, freeing 130 octets will likely make no difference at all.
Last of all, these 130 octets are needed during the works of the library. If the library were changed to allow freeing them, they still would be needed when it is in use. Actually, avoiding global state would likely increase the memory use marginally, due to the use of an extra indirection.
In short: you are barking at the wrong tree.
Note that I am all in favour of removing all global allocated state. But not due to valgrind's output, it is currently satisfactory. The good reason would be API cleanliness, and it would warrant the small extra memory requirement.
This all sounds to me as if you would argue that the "leak" (if there is a leak) could be fixed but it is not worth the effort.
I believe the "leak" (if it exists) cannot be fixed because there is no way to free the resources allocated by dl_open() from the process calling dl_open() - from valgrind's pov.
But maybe I am missing something and somebody can explain how I am wrong?
comment:14 by , 9 years ago
| Resolution: | → invalid |
|---|---|
| Status: | reopened → closed |
As far as I tested, this cannot be fixed (within FFmpeg): A process cannot free the resources allocated by a call to dlopen() - as far as valgrind is concerned at least.
Do you have any indication that this is an issue that can be fixed within FFmpeg?