[FFmpeg-trac] #6334(avformat:new): Memory leak: (RTSPState)->real_setup_cache

[FFmpeg]

#6334: Memory leak: (RTSPState)->real_setup_cache
----------------------------------+--------------------------------------
             Reporter:  skunk     |                     Type:  defect
               Status:  new       |                 Priority:  normal
            Component:  avformat  |                  Version:  git-master
             Keywords:            |               Blocked By:
             Blocking:            |  Reproduced by developer:  0
Analyzed by developer:  0         |
----------------------------------+--------------------------------------
 I am testing RTSP streaming of CCTV video via the FFmpeg API, and have
 found an intermittent memory leak using Valgrind.

 (Testing and line numbers are all w.r.t. ffmpeg Git revision 61088051,
 from 2017-04-16)

 {{{
 ==21497== 16 bytes in 1 blocks are definitely lost in loss record 3 of 16
 ==21497==    at 0x4C2DF16: memalign (vg_replace_malloc.c:857)
 ==21497==    by 0x4C2E021: posix_memalign (vg_replace_malloc.c:1020)
 ==21497==    by 0x612E103: av_malloc (mem.c:87)
 ==21497==    by 0x612E4B3: av_mallocz (mem.c:224)
 ==21497==    by 0x656E935: av_mallocz_array (mem.h:233)
 ==21497==    by 0x6571248: rtsp_read_header (rtspdec.c:731)
 ==21497==    by 0x65A6B07: avformat_open_input (utils.c:595)
 ==21497==    by 0x14839D: netcam_rtsp_open_context (netcam_rtsp.c:536)
 ==21497==    by 0x149044: netcam_connect_rtsp (netcam_rtsp.c:817)
 ==21497==    by 0x128D0B: netcam_handler_loop (netcam.c:1885)
 ==21497==    by 0x8985423: start_thread (pthread_create.c:333)
 ==21497==    by 0x8C839BE: clone (clone.S:105)
 }}}

 (Note: `netcam_rtsp_open_context()` on down is the application.)

 The 16-byte allocation in `rtsp_read_header()` is assigned to
 `rt->real_setup_cache`. Normally, this is freed in `rtsp_read_close()`
 when the RTSP connection is closed.

 However, my testing involves unreliable camera connectivity, and I am
 observing this chain of events:

 1. Application calls `avformat_open_input()`

 2. `avformat_open_input()` allocates `s->priv_data` (actually `RTSPState
 rt`) and then calls `s->iformat->read_header(s)` (actually
 `rtsp_read_header()`)

 3. `rtsp_read_header()` allocates `rt->real_setup_cache`

 4. Connection to RTSP host fails, `rtsp_read_header()` returns negative

 5. Back in `avformat_open_input()`, the error is caught, thus `goto fail`

 6. The `fail` cleanup code calls `avformat_free_context()`, which frees
 `s->priv_data`. But because `s->iformat->read_close()` (i.e.
 `rtsp_read_close()`) is not called, `real_setup_cache` is not freed.


 I tried adding this bit to the `fail` code:

 {{{
     if (s->iformat && s->iformat->read_close && s->priv_data)
         s->iformat->read_close(s);
 }}}

 Unfortunately, `rtsp_read_close()` assumes that there is an active
 connection (more specifically, that `rt->rtsp_hd` et al. are non-NULL), so
 this causes more Valgrind issues than it solves.

 It is possible that `rtsp_read_header()` could free this memory itself on
 error. However, there are a few more `goto fail` conditionals later in
 `avformat_open_input()`, and while these may or may not be applicable to
 `RTSPState`, they do suggest that a more general solution is needed.

--
Ticket URL: <https://trac.ffmpeg.org/ticket/6334>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker