From: Sangchul Lee Date: Tue, 12 Apr 2022 04:34:25 +0000 (+0900) Subject: webrtc_data_channel: Release data channel after receiving close callback X-Git-Tag: submit/tizen/20220426.020921~12 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F70%2F273670%2F4;p=platform%2Fcore%2Fapi%2Fwebrtc.git webrtc_data_channel: Release data channel after receiving close callback When destroying a data channel created by local peer, close callback could be invoked in the middle of the process. Due to the early disconnection signals, it'd never happen properly. Add 'from_remote' variable to check if it is created by _webrtcbin_on_data_channel_cb(). This kind of data channel can not be destroyed by webrtc_destroy_data_channel(). A FIXME comment is added in _webrtcbin_on_data_channel_cb(). [Version] 0.3.83 [Issue Type] Improvement Change-Id: Ic0bf5b3efc0760fe3221888cde038d5b1b4000fd Signed-off-by: Sangchul Lee --- diff --git a/include/webrtc_private.h b/include/webrtc_private.h index 05e9eb03..aeab58ba 100644 --- a/include/webrtc_private.h +++ b/include/webrtc_private.h @@ -542,6 +542,8 @@ typedef struct _webrtc_data_channel_s { GMutex mutex; GObject *channel; GList *signals; + bool removed_from_table; + bool from_remote; webrtc_callbacks_s open_cb; webrtc_callbacks_s message_cb; diff --git a/packaging/capi-media-webrtc.spec b/packaging/capi-media-webrtc.spec index 3288c67a..2e3aa646 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.82 +Version: 0.3.83 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/webrtc_data_channel.c b/src/webrtc_data_channel.c index 40a461b5..141324e3 100644 --- a/src/webrtc_data_channel.c +++ b/src/webrtc_data_channel.c @@ -86,6 +86,25 @@ static void __data_channel_on_error_cb(GObject *data_channel, GError *error, gpo } } +static void __release_data_channel(webrtc_data_channel_s *channel) +{ + RET_IF(channel == NULL, "channel is NULL"); + + g_mutex_lock(&channel->mutex); + + g_list_free_full(channel->signals, _disconnect_signal); + channel->signals = NULL; + + LOG_DEBUG("channel[%p, object:%p, webrtc:%p] is released", channel, channel->channel, channel->webrtc); + + gst_object_unref(channel->channel); + + g_mutex_unlock(&channel->mutex); + g_mutex_clear(&channel->mutex); + + g_free(channel); +} + static void __data_channel_on_close_cb(GObject *data_channel, gpointer user_data) { webrtc_data_channel_s *channel = (webrtc_data_channel_s *)user_data; @@ -99,6 +118,9 @@ static void __data_channel_on_close_cb(GObject *data_channel, gpointer user_data ((webrtc_data_channel_close_cb)(channel->close_cb.callback))((webrtc_data_channel_h)channel, channel->close_cb.user_data); LOG_DEBUG("<<< end of the callback"); } + + if (channel->removed_from_table) /* in case that it is in the middle of destroying handle */ + __release_data_channel(channel); } //LCOV_EXCL_STOP @@ -177,9 +199,13 @@ void _webrtcbin_on_data_channel_cb(GstElement *webrtcbin, GObject *data_channel, RET_IF(webrtc == NULL, "webrtc is NULL"); + /* FIXME: unify calling gst_object_ref() or not with in _create_data_channel() together + * after fixing webrtcbin issue. */ if (!(channel = __prepare_data_channel(webrtc, gst_object_ref(data_channel)))) return; + channel->from_remote = true; + LOG_INFO("data channel[%p, object:%p, label:%s] is created by remote request", channel, data_channel, GST_OBJECT_NAME(data_channel)); @@ -224,7 +250,7 @@ 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"); + RET_VAL_IF(channel->from_remote, WEBRTC_ERROR_INVALID_PARAMETER, "do not destroy a data channel obtained from _on_data_channel_cb()"); g_hash_table_remove(channel->webrtc->data_channels, channel); @@ -233,33 +259,24 @@ int _destroy_data_channel(webrtc_data_channel_s *channel) return WEBRTC_ERROR_NONE; } -static int __unprepare_data_channel(webrtc_data_channel_s *channel) +static bool __unprepare_data_channel(webrtc_data_channel_s *channel) { GstWebRTCDataChannelState state; - RET_VAL_IF(channel == NULL, WEBRTC_ERROR_INVALID_PARAMETER, "channel is NULL"); + RET_VAL_IF(channel == NULL, false, "channel is NULL"); g_mutex_lock(&channel->mutex); - g_object_get(channel->channel, "ready-state", &state, NULL); + channel->removed_from_table = true; + g_mutex_unlock(&channel->mutex); if (state == GST_WEBRTC_DATA_CHANNEL_STATE_OPEN) { LOG_DEBUG("request to close channel[%p, %p]", channel, channel->channel); + /* do __release_data_channel() in __data_channel_on_close_cb() */ g_signal_emit_by_name(channel->channel, "close"); + return false; } - g_list_free_full(channel->signals, _disconnect_signal); - channel->signals = NULL; - - 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); - - g_free(channel); - - return WEBRTC_ERROR_NONE; + return true; } static void __data_channel_destroy_cb(gpointer data) @@ -268,7 +285,8 @@ static void __data_channel_destroy_cb(gpointer data) RET_IF(data_channel == NULL, "data_channel is NULL"); - __unprepare_data_channel(data_channel); + if (__unprepare_data_channel(data_channel)) + __release_data_channel(data_channel); } void _init_data_channels(webrtc_s *webrtc) diff --git a/test/webrtc_test.c b/test/webrtc_test.c index 0b27b57b..5db19efb 100644 --- a/test/webrtc_test.c +++ b/test/webrtc_test.c @@ -562,6 +562,14 @@ static void _webrtc_destroy(int index) RET_IF(!g_conns[index].webrtc, "webrtc is NULL"); + for (i = 0; i < MAX_CHANNEL_LEN; i++) { + if (g_conns[index].channels[i]) { + webrtc_destroy_data_channel(g_conns[index].channels[i]); + g_conns[index].channels[i] = NULL; + } + g_conns[index].recv_channels[i] = NULL; + } + ret = webrtc_destroy(g_conns[index].webrtc); RET_IF(ret != WEBRTC_ERROR_NONE, "ret[0x%x]", ret); @@ -583,12 +591,6 @@ static void _webrtc_destroy(int index) g_list_free_full(g_conns[index].ice_candidates, free); g_conns[index].ice_candidates = NULL; - for (i = 0; i < MAX_CHANNEL_LEN; i++) { - if (g_conns[index].channels[i] != NULL) - g_conns[index].channels[i] = NULL; - if (g_conns[index].recv_channels[i] != NULL) - g_conns[index].recv_channels[i] = NULL; - } g_conns[index].channel_index = 0; for (i = 0; i < MAX_MEDIA_PACKET_SOURCE_LEN; i++) @@ -2604,17 +2606,8 @@ static void _webrtc_destroy_data_channel(int index) } g_conns[index].channel_index = 0; - for (i = 0; i < MAX_CHANNEL_LEN; i++) { - if (g_conns[index].recv_channels[i] == NULL) - continue; - ret = webrtc_destroy_data_channel(g_conns[index].recv_channels[i]); - if (ret != WEBRTC_ERROR_NONE) { - g_print("failed to _webrtc_destroy_data_channel(), index[%d]\n", i); - } else { - g_print("_webrtc_destroy_data_channel() success, receive channel index[%d]\n", i); - g_conns[index].recv_channels[i] = NULL; - } - } + for (i = 0; i < MAX_CHANNEL_LEN; i++) + g_conns[index].recv_channels[i] = NULL; } static void _webrtc_data_channel_get_label(int index)