Skip to content

dtls: Fixed memory leak, DtlsConnection not being correctly unreferenced

Saul Labajo requested to merge slabajo/gst-plugins-bad:master into master

We found a memory leak issue about DtlsConnection objects not being released. We traced the problem and found the cause for this in the following:

On function bio_method_write in gstdtlsconnection.c a closure is invoked with three values, the first one is a GstDtlsConnection object that is copied with g_value_set_object thus incrementing its reference count:

static int bio_method_write (BIO * bio, const char *data, int size)
{

  GstDtlsConnection *self = GST_DTLS_CONNECTION (BIO_get_data (bio));

  GST_LOG_OBJECT (self, "BIO: writing %d", size);

  if (self->priv->send_closure) {
    GValue values[3] = { G_VALUE_INIT };

    g_value_init (&values[0], GST_TYPE_DTLS_CONNECTION);
    g_value_set_object (&values[0], self);

    g_value_init (&values[1], G_TYPE_POINTER);
    g_value_set_pointer (&values[1], (gpointer) data);

    g_value_init (&values[2], G_TYPE_INT);
    g_value_set_int (&values[2], size);

    g_closure_invoke (self->priv->send_closure, NULL, 3, values, NULL);

    g_value_unset (&values[0]);
  }

  return size;
}

That closure is executed on the on_send_data function in gstdtlsenc.c as specified in the gst_dtls_enc_change_state function:

static GstStateChangeReturn
gst_dtls_enc_change_state (GstElement * element, GstStateChange transition)
{
     ……..
        gst_dtls_connection_set_send_callback (self->connection,
            g_cclosure_new (G_CALLBACK (on_send_data), self, NULL));
     ………..
}

The on_send_data function then receives the DtlsConnection with its reference count incremented, which I understand is correct, but once it finishes processing the reference count of DtlsConnection is not decreased. In our tests with a sample case (from a real production code base) we always observed the DtlsConnection object ending with a reference count of around 10 and thus not ever being finalized.

The proposed fix is pretty simple, just unref the DtlsConnection at the end of on_send_data function. We verified that no other code ever uses on_send_data so this change should have no impact apart from fixing the bug:

static void
on_send_data (GstDtlsConnection * connection, gconstpointer data, gint length,
    GstDtlsEnc * self)
{
   ……..
  GST_TRACE_OBJECT (self, "send data: releasing lock");
  g_mutex_unlock (&self->queue_lock);

  g_object_unref(connection);
}
Edited by Tim-Philipp Müller

Merge request reports