From: Sangchul Lee Date: Tue, 25 Oct 2022 06:12:00 +0000 (+0900) Subject: Check if handle is destroying before posting idle-callbacks X-Git-Tag: accepted/tizen/7.0/unified/20221115.022315^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3bc0d1755ed6b047538468d25e95d73c95f0e8f5;p=platform%2Fcore%2Fapi%2Fwebrtc.git Check if handle is destroying before posting idle-callbacks Sometimes, deadlock happens in C# TCT because of a sequence below. - webrtc handle seems to be destroyed before negotiation callbacks have not been processed in the main thread posted by g_idle_add_full(). It is checked that state and error callbacks have no chance to get this situation. [Version] 0.3.263 [Issue Type] Improvement Change-Id: Ia7359bb45aaa1095dd77ee673c5b3b72056da62b Signed-off-by: Sangchul Lee --- diff --git a/include/webrtc_private.h b/include/webrtc_private.h index 082b7580..115c4a76 100644 --- a/include/webrtc_private.h +++ b/include/webrtc_private.h @@ -482,6 +482,7 @@ typedef struct _webrtc_s { GCond desc_cond; GMutex stats_mutex; GCond stats_cond; + GMutex destroy_mutex; webrtc_gst_s gst; unsigned int payload_types; @@ -499,6 +500,7 @@ typedef struct _webrtc_s { webrtc_state_e state; webrtc_state_e pend_state; + bool is_destroying; GList *signals; diff --git a/packaging/capi-media-webrtc.spec b/packaging/capi-media-webrtc.spec index f59c3028..9ada9ba3 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.262 +Version: 0.3.263 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/webrtc.c b/src/webrtc.c index e5b50cf4..e971627a 100644 --- a/src/webrtc.c +++ b/src/webrtc.c @@ -98,6 +98,7 @@ int webrtc_create(webrtc_h *webrtc) g_cond_init(&_webrtc->desc_cond); g_mutex_init(&_webrtc->stats_mutex); g_cond_init(&_webrtc->stats_cond); + g_mutex_init(&_webrtc->destroy_mutex); _load_ini(_webrtc); if (_is_resource_required(&_webrtc->ini)) { @@ -137,16 +138,19 @@ int webrtc_destroy(webrtc_h webrtc) { int ret = WEBRTC_ERROR_NONE; g_autoptr(GMutexLocker) locker = NULL; + g_autoptr(GMutexLocker) d_locker = NULL; webrtc_s *_webrtc = (webrtc_s *)webrtc; RET_VAL_IF(_webrtc == NULL, WEBRTC_ERROR_INVALID_PARAMETER, "webrtc is NULL"); locker = g_mutex_locker_new(&_webrtc->mutex); + d_locker = g_mutex_locker_new(&_webrtc->destroy_mutex); - _remove_remained_event_sources(_webrtc); - + _webrtc->is_destroying = true; _webrtc->pend_state = WEBRTC_STATE_IDLE; + _remove_remained_event_sources(_webrtc); + _unset_stats_timer(_webrtc); ret = _gst_pipeline_set_state(_webrtc, GST_STATE_NULL); @@ -159,7 +163,10 @@ int webrtc_destroy(webrtc_h webrtc) _webrtc->state = _webrtc->pend_state; _destroy_data_channels(_webrtc); + + g_clear_pointer(&d_locker, g_mutex_locker_free); _gst_destroy_pipeline(_webrtc); + _unload_ini(_webrtc); #if !defined(TIZEN_TV) && defined(TIZEN_FEATURE_UI) @@ -175,6 +182,7 @@ int webrtc_destroy(webrtc_h webrtc) LOG_INFO("webrtc[%p] is destroyed", webrtc); + g_mutex_clear(&_webrtc->destroy_mutex); g_clear_pointer(&locker, g_mutex_locker_free); g_mutex_clear(&_webrtc->mutex); diff --git a/src/webrtc_private.c b/src/webrtc_private.c index 87bf64df..ac88c2aa 100644 --- a/src/webrtc_private.c +++ b/src/webrtc_private.c @@ -773,6 +773,7 @@ static void __post_peer_connection_state_change_cb_in_idle(webrtc_s *webrtc, web g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtc == NULL, "webrtc is NULL"); + RET_IF(webrtc->is_destroying, "webrtc is destroying, skip it"); data = g_new0(idle_userdata_s, 1); data->webrtc = webrtc; @@ -794,6 +795,7 @@ static void __post_signaling_state_change_cb_in_idle(webrtc_s *webrtc, webrtc_si g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtc == NULL, "webrtc is NULL"); + RET_IF(webrtc->is_destroying, "webrtc is destroying, skip it"); data = g_new0(idle_userdata_s, 1); data->webrtc = webrtc; @@ -814,6 +816,7 @@ static void __post_ice_gathering_state_change_cb_in_idle(webrtc_s *webrtc, webrt g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtc == NULL, "webrtc is NULL"); + RET_IF(webrtc->is_destroying, "webrtc is destroying, skip it"); data = g_new0(idle_userdata_s, 1); data->webrtc = webrtc; @@ -835,6 +838,7 @@ static void __post_ice_connection_state_change_cb_in_idle(webrtc_s *webrtc, webr g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtc == NULL, "webrtc is NULL"); + RET_IF(webrtc->is_destroying, "webrtc is destroying, skip it"); data = g_new0(idle_userdata_s, 1); data->webrtc = webrtc; @@ -1148,6 +1152,7 @@ static void __webrtcbin_peer_connection_state_cb(GstElement *webrtcbin, GParamSp { webrtc_s *webrtc = (webrtc_s *)user_data; GstWebRTCPeerConnectionState state; + g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtcbin == NULL, "webrtcbin is NULL"); RET_IF(webrtc == NULL, "webrtc is NULL"); @@ -1156,17 +1161,18 @@ static void __webrtcbin_peer_connection_state_cb(GstElement *webrtcbin, GParamSp LOG_DEBUG("webrtc[%p] [PeerConnectionState] is changed to [%s]", webrtc, __peer_connection_state_info[state].str); + locker = g_mutex_locker_new(&webrtc->destroy_mutex); + __post_peer_connection_state_change_cb_in_idle(webrtc, __peer_connection_state_info[state].state); switch (state) { case GST_WEBRTC_PEER_CONNECTION_STATE_CONNECTED: - g_mutex_lock(&webrtc->mutex); if (webrtc->state == WEBRTC_STATE_NEGOTIATING) _post_state_cb_in_idle(webrtc, WEBRTC_STATE_PLAYING); - g_mutex_unlock(&webrtc->mutex); break; case GST_WEBRTC_PEER_CONNECTION_STATE_FAILED: + g_clear_pointer(&locker, g_mutex_locker_free); __invoke_error_cb(webrtc, WEBRTC_ERROR_CONNECTION_FAILED); break; @@ -1180,6 +1186,7 @@ static void __webrtcbin_signaling_state_cb(GstElement *webrtcbin, GParamSpec * p { webrtc_s *webrtc = (webrtc_s *)user_data; GstWebRTCSignalingState state; + g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtcbin == NULL, "webrtcbin is NULL"); RET_IF(webrtc == NULL, "webrtc is NULL"); @@ -1192,6 +1199,8 @@ static void __webrtcbin_signaling_state_cb(GstElement *webrtcbin, GParamSpec * p state == GST_WEBRTC_SIGNALING_STATE_STABLE) __update_data_recovery_type_from_remote_description(webrtc); + locker = g_mutex_locker_new(&webrtc->destroy_mutex); + __post_signaling_state_change_cb_in_idle(webrtc, __signaling_state_info[state].state); } @@ -1199,6 +1208,7 @@ static void __webrtcbin_ice_gathering_state_cb(GstElement *webrtcbin, GParamSpec { webrtc_s *webrtc = (webrtc_s *)user_data; GstWebRTCICEGatheringState state; + g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtcbin == NULL, "webrtcbin is NULL"); RET_IF(webrtc == NULL, "webrtc is NULL"); @@ -1207,6 +1217,8 @@ static void __webrtcbin_ice_gathering_state_cb(GstElement *webrtcbin, GParamSpec LOG_DEBUG("webrtc[%p] [IceGatheringState] is changed to [%s]", webrtc, __ice_gathering_state_info[state].str); + locker = g_mutex_locker_new(&webrtc->destroy_mutex); + __post_ice_gathering_state_change_cb_in_idle(webrtc, __ice_gathering_state_info[state].state); } @@ -1215,6 +1227,7 @@ static void __webrtcbin_ice_connection_state_cb(GstElement *webrtcbin, GParamSpe { webrtc_s *webrtc = (webrtc_s *)user_data; GstWebRTCICEConnectionState state; + g_autoptr(GMutexLocker) locker = NULL; RET_IF(webrtcbin == NULL, "webrtcbin is NULL"); RET_IF(webrtc == NULL, "webrtc is NULL"); @@ -1223,6 +1236,8 @@ static void __webrtcbin_ice_connection_state_cb(GstElement *webrtcbin, GParamSpe LOG_DEBUG("webrtc[%p] [IceConnectionState] is changed to [%s]", webrtc, __ice_connection_state_info[state].str); + locker = g_mutex_locker_new(&webrtc->destroy_mutex); + __post_ice_connection_state_change_cb_in_idle(webrtc, __ice_connection_state_info[state].state); if (state == GST_WEBRTC_ICE_CONNECTION_STATE_FAILED)