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 <sc11.lee@samsung.com>

diff --git a/include/webrtc_private.h b/include/webrtc_private.h
index 05e9eb034df6a44586809275b2cd3173704802ce..aeab58bae6762fb5339567c7790ef23f92ca4c21 100644 (file)
--- 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 3288c67a90c8f2dcd1db7765844fba871b44ce3d..2e3aa64616a69ad3d8f6953ba12d9fac3537604e 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.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 40a461b5f4ffdf8b91258165c468f698fba7e7e9..141324e3c563ade90248ccd0f4fb5600180d44f7 100644 (file)
--- 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 0b27b57b7bfbfea5975b870bfce1829c88f04a38..5db19efbb2bd9827961000acebee13c12e793374 100644 (file)
--- 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)