From 089f478189cafef13d584e55f75445fc3432b8bc Mon Sep 17 00:00:00 2001 From: Sangchul Lee Date: Fri, 8 Apr 2022 14:24:28 +0900 Subject: [PATCH] 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 --- include/webrtc_private.h | 3 +- packaging/capi-media-webrtc.spec | 2 +- src/webrtc_data_channel.c | 55 +++++++++++++++++++++----------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/include/webrtc_private.h b/include/webrtc_private.h index e0b1ef13..05e9eb03 100644 --- 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 156f6ce9..3288c67a 100644 --- 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 7a21a148..40a461b5 100644 --- 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) -- 2.34.1