webrtc_data_channel: Fix memory leak and double free

g_object_unref() is added for data channel object.

When webrtc_destroy() is called, data channels appended to the data channel
list are also released. Due to the omitted code to remove one from the list
when calling webrtc_destroy_data_channel(), double free can occur.

The above are fixed now.

[Version] 0.3.82
[Issue Type] Bug fix

Change-Id: I2b2942666c8ab992bb2a7fe21fc9546b5bdd3019
Signed-off-by: Sangchul Lee <sc11.lee@samsung.com>

diff --git a/include/webrtc_private.h b/include/webrtc_private.h
index e0b1ef13dd8780fbd108dac53a0dcbe834e6252f..05e9eb034df6a44586809275b2cd3173704802ce 100644 (file)
--- a/include/webrtc_private.h
+++ b/include/webrtc_private.h
@@ -538,10 +538,9 @@ typedef struct _webrtc_gst_slot_s {
 } webrtc_gst_slot_s;
 
 typedef struct _webrtc_data_channel_s {
+       webrtc_s *webrtc;
        GMutex mutex;
-
        GObject *channel;
-
        GList *signals;
 
        webrtc_callbacks_s open_cb;

diff --git a/packaging/capi-media-webrtc.spec b/packaging/capi-media-webrtc.spec
index 156f6ce99a197ea7fec0db74c710a9e83e616ada..3288c67a90c8f2dcd1db7765844fba871b44ce3d 100644 (file)
--- a/packaging/capi-media-webrtc.spec
+++ b/packaging/capi-media-webrtc.spec
@@ -1,6 +1,6 @@
 Name:       capi-media-webrtc
 Summary:    A WebRTC library in Tizen Native API
-Version:    0.3.81
+Version:    0.3.82
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0

diff --git a/src/webrtc_data_channel.c b/src/webrtc_data_channel.c
index 7a21a14832be9af038ccd54144f51f684bc01705..40a461b5f4ffdf8b91258165c468f698fba7e7e9 100644 (file)
--- a/src/webrtc_data_channel.c
+++ b/src/webrtc_data_channel.c
@@ -135,18 +135,27 @@ static void __data_channel_on_buffered_amount_low_cb(GObject *data_channel, gpoi
 }
 //LCOV_EXCL_STOP
 
-static webrtc_data_channel_s *__prepare_data_channel(GObject *dc_obj)
+static webrtc_data_channel_s *__prepare_data_channel(webrtc_s *webrtc, GObject *dc_obj)
 {
        webrtc_data_channel_s *_channel;
 
+       RET_VAL_IF(webrtc == NULL, NULL, "webrtc is NULL");
        RET_VAL_IF(dc_obj == NULL, NULL, "dc_obj is NULL");
 
        _channel = g_new0(webrtc_data_channel_s, 1);
        g_mutex_init(&_channel->mutex);
        g_mutex_lock(&_channel->mutex);
 
+       _channel->webrtc = webrtc;
        _channel->channel = dc_obj;
 
+       if (!g_hash_table_insert(webrtc->data_channels, (gpointer)_channel, (gpointer)_channel)) {
+               LOG_ERROR("should not be reached here, _channel[%p] already exist, it'll be removed", _channel);
+               g_mutex_unlock(&_channel->mutex);
+               g_hash_table_remove(webrtc->data_channels, _channel);
+               return NULL;
+       }
+
        _connect_and_append_signal(&_channel->signals, dc_obj, "on-open", G_CALLBACK(__data_channel_on_open_cb), _channel);
        _connect_and_append_signal(&_channel->signals, dc_obj, "on-message-string", G_CALLBACK(__data_channel_on_message_string_cb), _channel);
        _connect_and_append_signal(&_channel->signals, dc_obj, "on-message-data", G_CALLBACK(__data_channel_on_message_data_cb), _channel);
@@ -154,6 +163,8 @@ static webrtc_data_channel_s *__prepare_data_channel(GObject *dc_obj)
        _connect_and_append_signal(&_channel->signals, dc_obj, "on-close", G_CALLBACK(__data_channel_on_close_cb), _channel);
        _connect_and_append_signal(&_channel->signals, dc_obj, "on-buffered-amount-low", G_CALLBACK(__data_channel_on_buffered_amount_low_cb), _channel);
 
+       LOG_DEBUG("channel[%p, object:%p, webrtc:%p] is prepared", _channel, dc_obj, webrtc);
+
        g_mutex_unlock(&_channel->mutex);
 
        return _channel;
@@ -166,17 +177,11 @@ void _webrtcbin_on_data_channel_cb(GstElement *webrtcbin, GObject *data_channel,
 
        RET_IF(webrtc == NULL, "webrtc is NULL");
 
-       if (!(channel = __prepare_data_channel(gst_object_ref(data_channel))))
+       if (!(channel = __prepare_data_channel(webrtc, gst_object_ref(data_channel))))
                return;
 
-       LOG_INFO("channel[%p, %p, %s] user_data[%p]", channel, data_channel, GST_OBJECT_NAME(data_channel), user_data);
-
-       if (!g_hash_table_insert(webrtc->data_channels, (gpointer)channel, (gpointer)channel)) {
-               LOG_ERROR("should not be reached here, channel[%p] already exist, it'll be removed", channel);
-               g_mutex_unlock(&channel->mutex);
-               g_hash_table_remove(webrtc->data_channels, channel);
-               return;
-       }
+       LOG_INFO("data channel[%p, object:%p, label:%s] is created by remote request",
+               channel, data_channel, GST_OBJECT_NAME(data_channel));
 
        g_mutex_lock(&webrtc->mutex);
        if (webrtc->state == WEBRTC_STATE_NEGOTIATING)
@@ -189,7 +194,7 @@ void _webrtcbin_on_data_channel_cb(GstElement *webrtcbin, GObject *data_channel,
 int _create_data_channel(webrtc_s *webrtc, const char *label, bundle *options, webrtc_data_channel_s **channel)
 {
        webrtc_data_channel_s *_channel;
-       GObject *data_channel;
+       GObject *dc_obj;
        GstStructure *_options = NULL;
 
        RET_VAL_IF(webrtc == NULL, WEBRTC_ERROR_INVALID_PARAMETER, "webrtc is NULL");
@@ -198,18 +203,18 @@ int _create_data_channel(webrtc_s *webrtc, const char *label, bundle *options, w
 
        _options = _get_structure_from_data_channel_options(options);
 
-       g_signal_emit_by_name(webrtc->gst.webrtcbin, "create-data-channel", label, _options, &data_channel);
+       g_signal_emit_by_name(webrtc->gst.webrtcbin, "create-data-channel", label, _options, &dc_obj);
        if (_options)
                gst_structure_free(_options);
-       if (!data_channel) {
-               LOG_ERROR("failed to create data channel");
+       if (!dc_obj) {
+               LOG_ERROR("failed to create-data-channel");
                return WEBRTC_ERROR_INVALID_OPERATION;
        }
 
-       if (!(_channel = __prepare_data_channel(data_channel)))
+       if (!(_channel = __prepare_data_channel(webrtc, dc_obj)))
                return WEBRTC_ERROR_INVALID_OPERATION;
 
-       LOG_INFO("channel[%p, %p, label:%s] is created", _channel, data_channel, label);
+       LOG_INFO("data channel[%p, object:%p, label:%s] is created", _channel, dc_obj, label);
 
        *channel = _channel;
 
@@ -217,6 +222,18 @@ int _create_data_channel(webrtc_s *webrtc, const char *label, bundle *options, w
 }
 
 int _destroy_data_channel(webrtc_data_channel_s *channel)
+{
+       RET_VAL_IF(channel == NULL, WEBRTC_ERROR_INVALID_PARAMETER, "channel is NULL");
+       RET_VAL_IF(channel->webrtc == NULL, WEBRTC_ERROR_INVALID_OPERATION, "webrtc is NULL");
+
+       g_hash_table_remove(channel->webrtc->data_channels, channel);
+
+       LOG_INFO("data channel[%p, object:%p] is destroyed", channel, channel->channel);
+
+       return WEBRTC_ERROR_NONE;
+}
+
+static int __unprepare_data_channel(webrtc_data_channel_s *channel)
 {
        GstWebRTCDataChannelState state;
 
@@ -233,7 +250,9 @@ int _destroy_data_channel(webrtc_data_channel_s *channel)
        g_list_free_full(channel->signals, _disconnect_signal);
        channel->signals = NULL;
 
-       LOG_INFO("channel[%p, %p] is destroyed", channel, channel->channel);
+       LOG_DEBUG("channel[%p, object:%p, webrtc:%p] is unprepared", channel, channel->channel, channel->webrtc);
+
+       gst_object_unref(channel->channel);
 
        g_mutex_unlock(&channel->mutex);
        g_mutex_clear(&channel->mutex);
@@ -249,7 +268,7 @@ static void __data_channel_destroy_cb(gpointer data)
 
        RET_IF(data_channel == NULL, "data_channel is NULL");
 
-       _destroy_data_channel(data_channel);
+       __unprepare_data_channel(data_channel);
 }
 
 void _init_data_channels(webrtc_s *webrtc)