Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

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

  • Keywords libx265 leak added

Do you have any indication that this is an issue that can be fixed within FFmpeg?

comment:2 Changed 4 years ago by dbuitenh

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 Changed 4 years ago by joeallen

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

(Not really sure why you'd care about something so minor, av_register_all() is a broken concept anyway.)

comment:5 Changed 4 years ago by joeallen

@gjdfgh

Good programming practices I and raise awareness..i guess. What did you mean by "av_register_all() is a broken concept?"

comment:6 Changed 4 years ago by gjdfgh

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

  • Resolution set to invalid
  • Status changed from new to closed

If I understand Alessandro Ghedini's analysis correctly, this is a false positive.

comment:8 Changed 4 years ago by rbultje

  • Resolution invalid deleted
  • Status changed from closed to 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 Changed 4 years ago by cehoyos

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;
}

comment:10 follow-ups: Changed 4 years ago by 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.

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 in reply to: ↑ 10 Changed 4 years ago by cehoyos

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.

comment:12 in reply to: ↑ 10 ; follow-up: Changed 4 years ago by 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 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 in reply to: ↑ 12 Changed 4 years ago by cehoyos

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

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

  • Resolution set to invalid
  • Status changed from reopened to 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.

Last edited 4 years ago by cehoyos (previous) (diff)
Note: See TracTickets for help on using tickets.