Avoid double free for idle event 54/122254/1
authorJeongmo Yang <jm80.yang@samsung.com>
Fri, 31 Mar 2017 01:37:46 +0000 (10:37 +0900)
committerJeongmo Yang <jm80.yang@samsung.com>
Fri, 31 Mar 2017 02:16:47 +0000 (11:16 +0900)
Sometimes, the idle callback is called although g_idle_remove_by_data returned true, then double free could be occurred.
So, update code to avoid it like below.
- remove g_idle_remove_by_data and idle callback will be always called
- remove free code for idle callback data in _recorder_remove_idle_event_all, then it will be freed in idle callback
- change function name from _recorder_remove_idle_event_all to _recorder_deactivate_idle_event_all

[Version] 0.2.53
[Profile] Common
[Issue Type] Update
[Dependency module] N/A
[Test] [M(T) - Boot=(OK), sdb=(OK), Home=(OK), Touch=(OK), Version=tizen-3.0-mobile_20170321.3]

Change-Id: I2a675fbb0cb0773d77bc956b7fe27292a11015a5
Signed-off-by: Jeongmo Yang <jm80.yang@samsung.com>
packaging/capi-media-recorder.spec
src/recorder.c

index 40aac24..e5a3eef 100644 (file)
@@ -1,7 +1,7 @@
 Name:       capi-media-recorder
 Summary:    A Recorder API
-Version:    0.2.52
-Release:    2
+Version:    0.2.53
+Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0
 Source0:    %{name}-%{version}.tar.gz
index 73480d6..4e09c86 100644 (file)
@@ -425,12 +425,13 @@ static bool _recorder_idle_event_callback(void *data)
 
        cb_info = rec_idle_event->cb_info;
        if (cb_info == NULL) {
-               LOGW("recorder cb_info is NULL. event %d", rec_idle_event->event);
+               LOGW("recorder cb_info is NULL. event %p %d", rec_idle_event, rec_idle_event->event);
                goto IDLE_EVENT_CALLBACK_DONE;
        }
 
        /* remove event from list */
        g_mutex_lock(&cb_info->idle_event_mutex);
+
        if (cb_info->idle_event_list)
                cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event);
 
@@ -438,7 +439,7 @@ static bool _recorder_idle_event_callback(void *data)
        g_mutex_unlock(&cb_info->idle_event_mutex);
 
        /* user callback */
-       _recorder_client_user_callback(rec_idle_event->cb_info, rec_idle_event->recv_msg, rec_idle_event->event);
+       _recorder_client_user_callback(cb_info, rec_idle_event->recv_msg, rec_idle_event->event);
 
        /* send signal for waiting thread */
        g_cond_signal(&cb_info->idle_event_cond);
@@ -455,10 +456,9 @@ IDLE_EVENT_CALLBACK_DONE:
 }
 
 
-static void _recorder_remove_idle_event_all(recorder_cb_info_s *cb_info)
+static void _recorder_deactivate_idle_event_all(recorder_cb_info_s *cb_info)
 {
        recorder_idle_event_s *rec_idle_event = NULL;
-       gboolean ret = true;
        GList *list = NULL;
        gint64 end_time = 0;
 
@@ -484,7 +484,7 @@ static void _recorder_remove_idle_event_all(recorder_cb_info_s *cb_info)
                        }
 
                        if (!g_mutex_trylock(&rec_idle_event->event_mutex)) {
-                               LOGW("lock failed");
+                               LOGW("lock failed, %p event is calling now", rec_idle_event);
 
                                end_time = g_get_monotonic_time() + G_TIME_SPAN_MILLISECOND * 100;
 
@@ -496,27 +496,14 @@ static void _recorder_remove_idle_event_all(recorder_cb_info_s *cb_info)
                                continue;
                        }
 
-                       ret = g_idle_remove_by_data(rec_idle_event);
-
-                       LOGD("remove event %p, ret %d", rec_idle_event, ret);
+                       LOGW("set NULL cb_info for event %p %d, it will be freed on idle callback",
+                               rec_idle_event, rec_idle_event->event);
 
-                       if (!ret) {
-                               rec_idle_event->cb_info = NULL;
-                               LOGW("idle cb for event %p will be called later", rec_idle_event);
-                       }
+                       rec_idle_event->cb_info = NULL;
 
                        cb_info->idle_event_list = g_list_remove(cb_info->idle_event_list, (gpointer)rec_idle_event);
 
                        g_mutex_unlock(&rec_idle_event->event_mutex);
-
-                       if (ret) {
-                               g_mutex_clear(&rec_idle_event->event_mutex);
-
-                               g_free(rec_idle_event);
-                               rec_idle_event = NULL;
-
-                               LOGD("remove event done");
-                       }
                }
 
                g_list_free(cb_info->idle_event_list);
@@ -1692,7 +1679,7 @@ int recorder_destroy(recorder_h recorder)
                LOGW("server disconnected. release resource without send message.");
 
        if (ret == RECORDER_ERROR_NONE) {
-               _recorder_remove_idle_event_all(pc->cb_info);
+               _recorder_deactivate_idle_event_all(pc->cb_info);
                _recorder_client_callback_destroy(pc->cb_info);
                g_free(pc);
                pc = NULL;