thread: use two lists for each thread 69/170769/2
authorBoram Park <boram1288.park@samsung.com>
Thu, 22 Feb 2018 05:02:51 +0000 (14:02 +0900)
committerSooChan Lim <sc1.lim@samsung.com>
Thu, 22 Feb 2018 06:13:15 +0000 (06:13 +0000)
When tdm_thread_call_cb is called in both threads at the same time, the 'called'
variable makes a thread issue.

1) Calling tdm_thread_call_cb in display-thread makes the 'called' variable as 1
2) Calling tdm_thread_call_cb in tdm-thread at the same time checks cb->called.
3) Because cb->called is 1 by display-thread, tdm_thread_call_cb in tdm-thread
   does nothing.

Change-Id: I009e3a17b40d32502f3567b8ecd712fa7c8dc349

src/tdm_thread.c

index 30966b4..6058bb3 100644 (file)
@@ -69,11 +69,13 @@ typedef struct _tdm_private_thread_cb {
        void *user_data;
 
        pid_t owner_tid;
-       unsigned int called;
 } tdm_private_thread_cb;
 
 static tdm_thread_find_object find_funcs[TDM_THREAD_CB_MAX] = {0, };
-static struct list_head cb_list;
+
+/* 0: for display thread, 1: for tdm thread */
+static struct list_head cb_list[2];
+static pthread_mutex_t cb_list_lock;
 
 static void _tdm_thread_free_cb(tdm_private_thread_cb *cb);
 
@@ -193,13 +195,19 @@ tdm_thread_init(tdm_private_loop *private_loop)
        private_display = private_loop->dpy;
        TDM_RETURN_VAL_IF_FAIL(private_display->private_loop, TDM_ERROR_OPERATION_FAILED);
 
+       if (private_loop->private_thread)
+               return TDM_ERROR_NONE;
+
        for (i = 0; i < TDM_THREAD_CB_MAX; i++)
                find_funcs[i] = NULL;
 
-       LIST_INITHEAD(&cb_list);
+       if (pthread_mutex_init(&cb_list_lock, NULL)) {
+               TDM_ERR("mutex init failed: %m");
+               return TDM_ERROR_OUT_OF_MEMORY;
+       }
 
-       if (private_loop->private_thread)
-               return TDM_ERROR_NONE;
+       LIST_INITHEAD(&cb_list[0]);
+       LIST_INITHEAD(&cb_list[1]);
 
        /* enable as default */
        thread = tdm_config_get_int(TDM_CONFIG_KEY_GENERAL_THREAD, 1);
@@ -291,7 +299,12 @@ tdm_thread_deinit(tdm_private_loop *private_loop)
 
        tdm_log_reset();
 
-       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, &cb_list, link) {
+       pthread_mutex_destroy(&cb_list_lock);
+
+       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, &cb_list[0], link) {
+               _tdm_thread_free_cb(cb);
+       }
+       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, &cb_list[1], link) {
                _tdm_thread_free_cb(cb);
        }
 
@@ -358,7 +371,7 @@ tdm_thread_send_cb(tdm_private_loop *private_loop, tdm_thread_cb_base *base)
                pipe = private_thread->pipe[1];
 
        if (tdm_debug_module & TDM_DEBUG_THREAD)
-               TDM_INFO("fd(%d) type(%d), length(%d)", pipe, base->type, base->length);
+               TDM_INFO("fd(%d) type(%s), length(%d)", pipe, tdm_cb_type_str(base->type), base->length);
 
        len = write(pipe, base, base->length);
        if (len != base->length) {
@@ -419,7 +432,7 @@ tdm_thread_handle_cb(tdm_private_loop *private_loop)
        while (i < len) {
                base = (tdm_thread_cb_base*)&buffer[i];
                if (tdm_debug_module & TDM_DEBUG_THREAD)
-                       TDM_INFO("type(%d), length(%d)", base->type, base->length);
+                       TDM_INFO("type(%s), length(%d)", tdm_cb_type_str(base->type), base->length);
                switch (base->type) {
                case TDM_THREAD_CB_OUTPUT_COMMIT:
                case TDM_THREAD_CB_OUTPUT_VBLANK:
@@ -472,34 +485,24 @@ _tdm_thread_free_cb(tdm_private_thread_cb *cb)
 }
 
 static tdm_private_thread_cb *
-_tdm_thread_find_cb(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_thread_cb func, void *user_data, pid_t owner_tid)
+_tdm_thread_find_cb(struct list_head *list, void *object, tdm_thread_cb_type cb_type,
+                                       void *cb_data, tdm_thread_cb func, void *user_data, pid_t caller_tid)
 {
        tdm_private_thread_cb *cb = NULL;
 
-       LIST_FOR_EACH_ENTRY(cb, &cb_list, link) {
+       LIST_FOR_EACH_ENTRY(cb, list, link) {
                if (cb->object == object &&
                        cb->cb_type == cb_type &&
                        cb->cb_data == cb_data &&
                        cb->func == func &&
                        cb->user_data == user_data &&
-                       cb->owner_tid == owner_tid)
+                       cb->owner_tid == caller_tid)
                        return cb;
        }
 
        return NULL;
 }
 
-static void
-_tdm_thread_reset_cb(tdm_thread_cb_type cb_type)
-{
-       tdm_private_thread_cb *cb = NULL;
-
-       LIST_FOR_EACH_ENTRY(cb, &cb_list, link) {
-               if (cb->cb_type == cb_type)
-                       cb->called = 0;
-       }
-}
-
 INTERN void
 tdm_thread_cb_set_find_func(tdm_thread_cb_type cb_type, tdm_thread_find_object func)
 {
@@ -516,6 +519,7 @@ tdm_thread_cb_add(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_t
 {
        tdm_private_thread_cb *cb = NULL;
        pid_t caller_tid;
+       struct list_head *list;
 
        TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED);
        TDM_RETURN_VAL_IF_FAIL(object != NULL, TDM_ERROR_INVALID_PARAMETER);
@@ -524,8 +528,16 @@ tdm_thread_cb_add(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_t
 
        caller_tid = syscall(SYS_gettid);
 
-       cb = _tdm_thread_find_cb(object, cb_type, cb_data, func, user_data, caller_tid);
+       pthread_mutex_lock(&cb_list_lock);
+
+       if (tdm_thread_in_display_thread(caller_tid))
+               list = &cb_list[0];
+       else
+               list = &cb_list[1];
+
+       cb = _tdm_thread_find_cb(list, object, cb_type, cb_data, func, user_data, caller_tid);
        if (cb) {
+               pthread_mutex_unlock(&cb_list_lock);
                TDM_ERR("can't be added twice with same data");
 #if 1
                assert(0);
@@ -535,11 +547,12 @@ tdm_thread_cb_add(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_t
 
        cb = calloc(1, sizeof *cb);
        if (!cb) {
+               pthread_mutex_unlock(&cb_list_lock);
                TDM_ERR("calloc failed");
                return TDM_ERROR_OUT_OF_MEMORY;
        }
 
-       LIST_ADDTAIL(&cb->link, &cb_list);
+       LIST_ADDTAIL(&cb->link, list);
        LIST_INITHEAD(&cb->call_link);
 
        cb->object = object;
@@ -550,7 +563,9 @@ tdm_thread_cb_add(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_t
        cb->owner_tid = caller_tid;
 
        if (tdm_debug_module & TDM_DEBUG_THREAD)
-               TDM_INFO("cb_type(%d) cb(%p) added", cb_type, cb);
+               TDM_INFO("cb_type(%s) cb(%p) added", tdm_cb_type_str(cb_type), cb);
+
+       pthread_mutex_unlock(&cb_list_lock);
 
        return TDM_ERROR_NONE;
 }
@@ -560,6 +575,7 @@ tdm_thread_cb_remove(void *object, tdm_thread_cb_type cb_type, void *cb_data, td
 {
        tdm_private_thread_cb *cb;
        pid_t caller_tid;
+       struct list_head *list;
 
        TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED());
        TDM_RETURN_IF_FAIL(object != NULL);
@@ -568,11 +584,21 @@ tdm_thread_cb_remove(void *object, tdm_thread_cb_type cb_type, void *cb_data, td
 
        caller_tid = syscall(SYS_gettid);
 
-       cb = _tdm_thread_find_cb(object, cb_type, cb_data, func, user_data, caller_tid);
-       if (!cb)
+       pthread_mutex_lock(&cb_list_lock);
+
+       if (tdm_thread_in_display_thread(caller_tid))
+               list = &cb_list[0];
+       else
+               list = &cb_list[1];
+
+       cb = _tdm_thread_find_cb(list, object, cb_type, cb_data, func, user_data, caller_tid);
+       if (!cb) {
+               pthread_mutex_unlock(&cb_list_lock);
                return;
+       }
 
        _tdm_thread_free_cb(cb);
+       pthread_mutex_unlock(&cb_list_lock);
 }
 
 /* when call a callback, we check both cb_base's type and cb_base's data,
@@ -585,6 +611,7 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base)
        tdm_private_thread_cb *cb = NULL, *hh = NULL;
        int handler_in_other_thread = 0;
        pid_t caller_tid;
+       struct list_head *list, *other_list;
        struct list_head call_list;
        tdm_error ret;
 
@@ -607,35 +634,53 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base)
                }
        }
 
+       pthread_mutex_lock(&cb_list_lock);
+
+       if (tdm_thread_in_display_thread(caller_tid)) {
+               list = &cb_list[0];
+               other_list = &cb_list[1];
+       } else {
+               other_list = &cb_list[0];
+               list = &cb_list[1];
+       }
+
        LIST_INITHEAD(&call_list);
 
-       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, &cb_list, link) {
-               if (cb->called ||
-                       cb->object != object ||
+       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, list, link) {
+               if (cb->object != object ||
                        cb->cb_type != cb_base->type ||
                        cb->cb_data != cb_base->data)
                        continue;
 
-               if (cb->owner_tid == caller_tid)
-                       LIST_ADDTAIL(&cb->call_link, &call_list);
-               else
-                       handler_in_other_thread = 1;
+               LIST_ADDTAIL(&cb->call_link, &call_list);
        }
 
        if (!LIST_IS_EMPTY(&call_list)) {
                LIST_FOR_EACH_ENTRY_SAFE(cb, hh, &call_list, call_link) {
                        LIST_DELINIT(&cb->call_link);
-                       cb->called = 1;
                        if (tdm_debug_module & TDM_DEBUG_THREAD)
-                               TDM_INFO("cb_type(%d) cb(%p) called", cb->cb_type, cb);
+                               TDM_INFO("cb_type(%s) cb(%p) calling", tdm_cb_type_str(cb->cb_type), cb);
+                       pthread_mutex_unlock(&cb_list_lock);
                        cb->func(private_display, cb->object, cb_base, cb->user_data);
+                       pthread_mutex_lock(&cb_list_lock);
                }
        }
 
+       pthread_mutex_unlock(&cb_list_lock);
+
        assert(LIST_IS_EMPTY(&call_list));
 
+       LIST_FOR_EACH_ENTRY_SAFE(cb, hh, other_list, link) {
+               if (cb->object != object ||
+                       cb->cb_type != cb_base->type ||
+                       cb->cb_data != cb_base->data)
+                       continue;
+
+               handler_in_other_thread = 1;
+               break;
+       }
+
        if (!handler_in_other_thread) {
-               _tdm_thread_reset_cb(cb_base->type);
                if (keep_private_thread) {
                        if (cb_base->sync) {
                                pthread_cond_signal(&keep_private_thread->event_cond);