From 0b21117e6457d0ae6a144f5cc2266c53eac19e33 Mon Sep 17 00:00:00 2001 From: Eunhye Choi Date: Tue, 18 May 2021 20:01:36 +0900 Subject: [PATCH] [0.3.141] resolve crash about event data - crash caused by invalid event_data access - release event_data resource by the destroy notify callback which is called when the GSource is removed. Change-Id: Ic9890709382789cecdef4667a456b0e9e0cfca6c --- packaging/capi-media-player.spec | 2 +- src/player.c | 102 ++++++++++++--------------------------- 2 files changed, 33 insertions(+), 71 deletions(-) diff --git a/packaging/capi-media-player.spec b/packaging/capi-media-player.spec index f67fb1d..238f946 100644 --- a/packaging/capi-media-player.spec +++ b/packaging/capi-media-player.spec @@ -1,6 +1,6 @@ Name: capi-media-player Summary: A Media Player API -Version: 0.3.140 +Version: 0.3.141 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/player.c b/src/player.c index 70a2c00..0466bb4 100644 --- a/src/player.c +++ b/src/player.c @@ -60,7 +60,6 @@ typedef struct { int int_data; callback_cb_info_s *cb_info; _player_recv_data *recv_data; - GMutex event_mutex; } _player_cb_data; typedef struct { @@ -1322,8 +1321,8 @@ gboolean _player_event_job_function(void *user_data) _player_cb_data *data = (_player_cb_data *)user_data; muse_player_event_e ev; - if (data == NULL) { - LOGE("data is null"); + if (!data || !data->cb_info) { + LOGE("invalid user data"); return FALSE; } @@ -1332,14 +1331,6 @@ gboolean _player_event_job_function(void *user_data) (ev != MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_AUDIO_FRAME)) LOGD("enter ev:%d(%p)", ev, data); - g_mutex_lock(&data->event_mutex); - - if (data->cb_info == NULL) { - /* tried to remove before at _player_remove_idle_event */ - LOGW("cb_info is NULL. event %d", data->int_data); - goto DONE; - } - /* remove event from list */ g_mutex_lock(&data->cb_info->event_queue.idle_ev_mutex); if (data->cb_info->event_queue.idle_ev_list) { @@ -1354,18 +1345,6 @@ gboolean _player_event_job_function(void *user_data) else LOGW("user callback is unset. type : %d", ev); -DONE: - /* unlock and release event */ - g_mutex_unlock(&data->event_mutex); - g_mutex_clear(&data->event_mutex); - if (data->recv_data) { - g_free(data->recv_data->buffer); - data->recv_data->buffer = NULL; - g_free(data->recv_data); - data->recv_data = NULL; - } - g_free(data); - return FALSE; /* remove from the event list */ } @@ -1384,6 +1363,26 @@ static bool _player_need_sync_context(int event_id) } } +static void _destroy_notify_cb(gpointer user_data) +{ + _player_cb_data *data = (_player_cb_data *)user_data; + + if (data == NULL) { + LOGW("data is null"); + return; + } + + LOGD("remove event data %p (%d)", data, data->int_data); + if (data->recv_data) { + g_free(data->recv_data->buffer); + data->recv_data->buffer = NULL; + g_free(data->recv_data); + data->recv_data = NULL; + } + + g_free(data); +} + static void *_player_event_queue_loop(void *param) { if (!param) { @@ -1443,7 +1442,7 @@ static void *_player_event_queue_loop(void *param) g_idle_add_full(G_PRIORITY_DEFAULT, (GSourceFunc)_player_event_job_function, (gpointer)event_data, - NULL); + (GDestroyNotify)_destroy_notify_cb); } else { LOGW("there is no registered cb for ev:%d", event_type); if (event_data->recv_data) { @@ -1515,54 +1514,19 @@ static void _player_remove_idle_event(callback_cb_info_s *cb_info, muse_player_e continue; } - if (g_mutex_trylock(&event_data->event_mutex)) { - - gboolean ret = FALSE; - if (remove_all || (event_data->int_data == event_type)) { - GSource *source = NULL; - gboolean check_in_call = FALSE; - - LOGD("remove idle event [%p:%d]", event_data, event_data->int_data); - source = g_main_context_find_source_by_user_data ( - g_main_context_default (), event_data); - if (source) - check_in_call = source->flags & G_HOOK_FLAG_IN_CALL; - - ret = g_idle_remove_by_data(event_data); - if (!ret || check_in_call) { - /* will be handled at _player_event_job_function() as an exception */ - event_data->cb_info = NULL; - LOGW("failed to remove, idle callback will be called later"); - } - - /* set cb to null */ - if (remove_all) - set_null_user_cb(cb_info, event_data->int_data); + if ((event_data->int_data != event_type) && !remove_all) + continue; - ev->idle_ev_list = g_list_remove(ev->idle_ev_list, (gpointer)event_data); + LOGD("remove idle event [%p:%d]", event_data, event_data->int_data); - g_mutex_unlock(&event_data->event_mutex); + /* set cb to null */ + if (remove_all) + set_null_user_cb(cb_info, event_data->int_data); - if (ret && !check_in_call) { - g_mutex_clear(&event_data->event_mutex); - if (event_data->recv_data) { - g_free(event_data->recv_data->buffer); - event_data->recv_data->buffer = NULL; - g_free(event_data->recv_data); - event_data->recv_data = NULL; - } - g_free(event_data); - event_data = NULL; - LOGD("remove idle event done"); - } /* else : will be handled if the cb is called. */ - } else { - g_mutex_unlock(&event_data->event_mutex); - } - } else { - LOGW("event(%d) lock failed. it's being called...", event_data->int_data); - } + ev->idle_ev_list = g_list_remove(ev->idle_ev_list, (gpointer)event_data); - /* continue: keep checking next event_data */ + if (!g_idle_remove_by_data(event_data)) + LOGW("failed to find source with %p", event_data); } if (remove_all) { @@ -1656,7 +1620,6 @@ static bool _user_callback_handler(callback_cb_info_s *cb_info, muse_player_even data->int_data = (int)event; data->cb_info = cb_info; data->recv_data = recv_data; - g_mutex_init(&data->event_mutex); _player_event_queue_add(&cb_info->event_queue, data); return true; @@ -1736,7 +1699,6 @@ static void _notify_disconnected(callback_cb_info_s *cb_info) data->int_data = (int)event; data->cb_info = cb_info; data->recv_data = NULL; - g_mutex_init(&data->event_mutex); _player_event_queue_add(&cb_info->event_queue, data); } } -- 2.7.4