From: Jeongmo Yang Date: Wed, 6 Nov 2024 00:18:23 +0000 (+0900) Subject: Add defensive code to avoid crash X-Git-Tag: accepted/tizen/unified/20241112.160418^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a906da3ae42165ead2042044cc046fa13c69ebd3;p=platform%2Fcore%2Fapi%2Frecorder.git Add defensive code to avoid crash - The user callback can be called in _recorder_idle_event_callback() although handle is destroyed after g_rec_idle_event_lock is unlocked. - Add new mutex and flag to avoid that situation. [Version] 1.0.1 [Issue Type] Improvement Change-Id: I6e19239b71c9b41d05c730f9ecd0d4c84c08c682 Signed-off-by: Jeongmo Yang --- diff --git a/include/recorder_private.h b/include/recorder_private.h index b5937c2..365c19e 100644 --- a/include/recorder_private.h +++ b/include/recorder_private.h @@ -147,6 +147,7 @@ typedef struct _recorder_cb_info_s { /* idle event */ GList *idle_event_list; + gboolean is_in_idle_cb; /* user callback */ gpointer user_cb[MUSE_RECORDER_EVENT_TYPE_NUM]; diff --git a/packaging/capi-media-recorder.spec b/packaging/capi-media-recorder.spec index fa683d9..b5f743c 100644 --- a/packaging/capi-media-recorder.spec +++ b/packaging/capi-media-recorder.spec @@ -1,7 +1,7 @@ Name: capi-media-recorder Summary: A Recorder API -Version: 1.0.0 -Release: 1 +Version: 1.0.1 +Release: 0 Group: Multimedia/API License: Apache-2.0 Source0: %{name}-%{version}.tar.gz diff --git a/src/recorder.c b/src/recorder.c index 9066219..967bdb8 100644 --- a/src/recorder.c +++ b/src/recorder.c @@ -588,7 +588,6 @@ static gboolean _recorder_idle_event_callback(gpointer data) cb_info = rec_idle_event->cb_info; if (cb_info == NULL) { REC_LOG_WARNING("recorder cb_info is NULL. event[%p][%d]", rec_idle_event, rec_idle_event->event); - g_mutex_unlock(&g_rec_idle_event_lock); goto IDLE_EVENT_CALLBACK_DONE; } @@ -596,15 +595,22 @@ static gboolean _recorder_idle_event_callback(gpointer data) if (cb_info->idle_event_list) cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event); + cb_info->is_in_idle_cb = TRUE; + g_mutex_unlock(&g_rec_idle_event_lock); /* user callback */ _recorder_client_user_callback(cb_info, rec_idle_event->recv_msg, rec_idle_event->event, NULL); + g_mutex_lock(&g_rec_idle_event_lock); + + cb_info->is_in_idle_cb = FALSE; + IDLE_EVENT_CALLBACK_DONE: + g_mutex_unlock(&g_rec_idle_event_lock); + /* release event */ g_free(rec_idle_event); - rec_idle_event = NULL; return FALSE; } @@ -620,44 +626,32 @@ static void _recorder_deactivate_idle_event_all(recorder_cb_info_s *cb_info) return; } - g_mutex_lock(&g_rec_idle_event_lock); - if (cb_info->idle_event_list == NULL) { - REC_LOG_INFO("No event"); - } else { - list = cb_info->idle_event_list; - - while (list) { - rec_idle_event = list->data; - list = g_list_next(list); - - if (!rec_idle_event) { - REC_LOG_WARNING("The event is NULL"); - continue; - } - - if (g_idle_remove_by_data(rec_idle_event)) { - REC_LOG_WARNING("remove idle event[%p] done", rec_idle_event); + REC_LOG_INFO("No remained idle event"); + return; + } - cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event); - g_free(rec_idle_event); + list = cb_info->idle_event_list; - continue; - } + while (list) { + rec_idle_event = list->data; + list = g_list_next(list); - REC_LOG_WARNING("set NULL cb_info for event[%p][%d], it will be freed on idle callback", - rec_idle_event, rec_idle_event->event); + if (!rec_idle_event) { + REC_LOG_WARNING("The event is NULL"); + continue; + } - rec_idle_event->cb_info = NULL; + REC_LOG_WARNING("set NULL cb_info for event[%d] %p, it will be freed on idle callback", + rec_idle_event->event, rec_idle_event); - cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event); - } + rec_idle_event->cb_info = NULL; - g_list_free(cb_info->idle_event_list); - cb_info->idle_event_list = NULL; + cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event); } - g_mutex_unlock(&g_rec_idle_event_lock); + g_list_free(cb_info->idle_event_list); + cb_info->idle_event_list = NULL; return; } @@ -925,7 +919,7 @@ static void __recorder_process_msg(recorder_cb_info_s *cb_info, char *msg, int * cb_info->api_activating[api]++; - g_cond_signal(&cb_info->api_cond[api]); + g_cond_broadcast(&cb_info->api_cond[api]); } else { REC_LOG_ERROR("no waiting for api[%d]", api); } @@ -1004,7 +998,7 @@ static void *_recorder_msg_handler_func(gpointer data) REC_LOG_DEBUG("recorder api[%d] - return[0x%x]", api, ret); - g_cond_signal(&cb_info->api_cond[api]); + g_cond_broadcast(&cb_info->api_cond[api]); } else { REC_LOG_ERROR("no waiting for api [%d]", api); } @@ -1137,6 +1131,11 @@ static void *_recorder_msg_recv_func(gpointer data) muse_core_msg_free(error_msg); error_msg = NULL; + for (i = 0 ; i < MUSE_RECORDER_API_MAX ; i++) { + if (cb_info->api_waiting[i]) + g_cond_broadcast(&cb_info->api_cond[i]); + } + REC_LOG_ERROR("add error msg for service disconnection done"); //LCOV_EXCL_STOP } @@ -1336,6 +1335,11 @@ static int _recorder_client_wait_for_cb_return(muse_recorder_api_e api, recorder goto _CB_RETURN_END; } + if (!cb_info->is_server_connected) { + REC_LOG_ERROR("server is disconnected"); + return RECORDER_ERROR_SERVICE_DISCONNECTED; + } + if (cb_info->api_activating[api] <= 0) REC_LOG_WARNING("api[%d] invalid signal received, wait again...", api); } @@ -1505,9 +1509,6 @@ static void _recorder_client_callback_destroy(recorder_cb_info_s *cb_info) g_return_if_fail(cb_info != NULL); - /* remove idle event */ - _recorder_deactivate_idle_event_all(cb_info); - REC_LOG_INFO("MSG receive thread[%p] destroy", cb_info->msg_recv_thread); g_thread_join(cb_info->msg_recv_thread); @@ -1874,6 +1875,7 @@ int recorder_destroy(recorder_h recorder) int ret = RECORDER_ERROR_NONE; muse_recorder_api_e api = MUSE_RECORDER_API_DESTROY; recorder_cli_s *pc = (recorder_cli_s *)recorder; + g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&g_rec_idle_event_lock); if (!pc || !pc->cb_info) { REC_LOG_ERROR("NULL handle"); @@ -1882,18 +1884,30 @@ int recorder_destroy(recorder_h recorder) REC_LOG_INFO("ENTER"); + if (pc->cb_info->is_in_idle_cb) { + REC_LOG_ERROR("should not be called in idle callback"); + return RECORDER_ERROR_INVALID_OPERATION; + } + if (pc->cb_info->is_server_connected) _recorder_msg_send(api, pc->cb_info, &ret, RECORDER_CB_TIMEOUT); else REC_LOG_WARNING("server disconnected. release resource without send message."); - if (ret == RECORDER_ERROR_NONE) { - _recorder_client_callback_destroy(pc->cb_info); - g_free(pc); - pc = NULL; + if (ret != RECORDER_ERROR_NONE) { + REC_LOG_ERROR("handle destroy failed[0x%x]", ret); + return ret; } - REC_LOG_INFO("ret[0x%x]", ret); + _recorder_deactivate_idle_event_all(pc->cb_info); + + g_clear_pointer(&locker, g_mutex_locker_free); + + _recorder_client_callback_destroy(pc->cb_info); + + g_free(pc); + + REC_LOG_INFO("done"); return ret; }