[FFmpeg-devel] ffserver: fix memory leaks pointed out by valgrind.

Submitted by Zalewa on April 16, 2017, 11:28 a.m.

Details

Message ID e3fa6513-d255-5dd9-d53b-7143a2723b0e@gmail.com
State New
Headers show

Commit Message

Zalewa April 16, 2017, 11:28 a.m.
Hello,

Approach 2.

This one reverses the shared state created by unlayer_stream() by 
nullifying the pointers to shared objects and then passes the "cleaned" 
AVFormatContext to avformat_free_context(). In result we have less code 
and less meddling with internals. See close_unlayered_format_context() 
function in ffserver.c.

The drawback of this approach is that I also had to modify free_stream() 
in libavformat/utils.c because it was not expecting that the "codec" 
pointer in AVStream can be null.

Let me know what you think.

Best regards,
Z.
From 4af94953f03989447e10f9aae013b4afb158c2e0 Mon Sep 17 00:00:00 2001
From: Zalewa <zalewapl@gmail.com>
Date: Fri, 14 Apr 2017 09:26:18 +0200
Subject: [PATCH] ffserver: fix memory leaks pointed out by valgrind.

Many memory leaks were created upon HTTP client disconnect.
Many clients connecting & disconnecting rapidly could very
quickly create leaks going into Gigabytes of memory.
---
 ffserver.c          | 40 +++++++++++++++++++++++-----------------
 libavformat/utils.c |  8 +++++---
 2 files changed, 28 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/ffserver.c b/ffserver.c
index 8b819b6..1748d81 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -270,6 +270,22 @@  static void unlayer_stream(AVStream *st, LayeredAVStream *lst)
     COPY(recommended_encoder_configuration)
 }
 
+/* NULLify all shared state that was applied in unlayer_stream. */
+static void detach_unlayered_stream(AVStream *st)
+{
+    st->codec = 0;
+    st->codecpar = 0;
+    st->recommended_encoder_configuration = 0;
+}
+
+static void close_unlayered_format_context(AVFormatContext *ctx)
+{
+    int i = 0;
+    for (i = 0; i < ctx->nb_streams; ++i)
+        detach_unlayered_stream(ctx->streams[i]);
+    avformat_free_context(ctx);
+}
+
 static inline void cp_html_entity (char *buffer, const char *entity) {
     if (!buffer || !entity)
         return;
@@ -936,9 +952,7 @@  static void close_connection(HTTPContext *c)
         ctx = c->rtp_ctx[i];
         if (ctx) {
             av_write_trailer(ctx);
-            av_dict_free(&ctx->metadata);
-            av_freep(&ctx->streams[0]);
-            av_freep(&ctx);
+            avformat_free_context(ctx);
         }
         ffurl_close(c->rtp_handles[i]);
     }
@@ -954,11 +968,9 @@  static void close_connection(HTTPContext *c)
                 avio_close_dyn_buf(ctx->pb, &c->pb_buffer);
             }
         }
-        for(i=0; i<ctx->nb_streams; i++)
-            av_freep(&ctx->streams[i]);
-        av_freep(&ctx->streams);
-        av_freep(&ctx->priv_data);
-        }
+        close_unlayered_format_context(ctx);
+        c->pfmt_ctx = 0;
+    }
 
     if (c->stream && !c->post && c->stream->stream_type == STREAM_TYPE_LIVE)
         current_bandwidth -= c->stream->bandwidth;
@@ -3836,7 +3848,7 @@  drop:
             }
             s->oformat = feed->fmt;
             for (i = 0; i<feed->nb_streams; i++) {
-                AVStream *st = avformat_new_stream(s, NULL); // FIXME free this
+                AVStream *st = avformat_new_stream(s, NULL);
                 if (!st) {
                     http_log("Failed to allocate stream\n");
                     goto bail;
@@ -3846,17 +3858,11 @@  drop:
             if (avformat_write_header(s, NULL) < 0) {
                 http_log("Container doesn't support the required parameters\n");
                 avio_closep(&s->pb);
-                s->streams = NULL;
-                s->nb_streams = 0;
-                avformat_free_context(s);
+                close_unlayered_format_context(s);
                 goto bail;
             }
-            /* XXX: need better API */
-            av_freep(&s->priv_data);
             avio_closep(&s->pb);
-            s->streams = NULL;
-            s->nb_streams = 0;
-            avformat_free_context(s);
+            close_unlayered_format_context(s);
         }
 
         /* get feed size and write index */
diff --git a/libavformat/utils.c b/libavformat/utils.c
index ba82a76..a392743 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4266,9 +4266,11 @@  static void free_stream(AVStream **pst)
     av_freep(&st->index_entries);
 #if FF_API_LAVF_AVCTX
 FF_DISABLE_DEPRECATION_WARNINGS
-    av_freep(&st->codec->extradata);
-    av_freep(&st->codec->subtitle_header);
-    av_freep(&st->codec);
+    if (st->codec) {
+        av_freep(&st->codec->extradata);
+        av_freep(&st->codec->subtitle_header);
+        av_freep(&st->codec);
+    }
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
     av_freep(&st->priv_data);