[0.3.141] resolve crash about event data 39/258739/1 accepted/tizen/unified/20210602.122630 submit/tizen/20210531.015432
authorEunhye Choi <eunhae1.choi@samsung.com>
Tue, 18 May 2021 11:01:36 +0000 (20:01 +0900)
committerEunhye Choi <eunhae1.choi@samsung.com>
Tue, 25 May 2021 04:43:44 +0000 (13:43 +0900)
- 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
src/player.c

index f67fb1d..238f946 100644 (file)
@@ -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
index 70a2c00..0466bb4 100644 (file)
@@ -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);
        }
 }