From d8df1d82aac258eb4ac81829e61e73796073ab4f Mon Sep 17 00:00:00 2001 From: Boram Park Date: Fri, 16 Mar 2018 10:17:28 +0900 Subject: [PATCH] thread: correct the infinite event propagation issue when calling tdm_thread_cb_call, 'propagation' param will help to decide if we need to propagate a event to other thread or not. and checking waiting_cb_type variable is not needed any more. Change-Id: I59929cb8dc1dd1ada2e7b8acfa9095a174e12928 --- src/tdm_capture.c | 2 +- src/tdm_output.c | 10 +++++----- src/tdm_pp.c | 2 +- src/tdm_thread.c | 43 +++++++++++++++++++++++++------------------ src/tdm_thread.h | 2 +- src/tdm_vblank.c | 4 ++-- 6 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/tdm_capture.c b/src/tdm_capture.c index 982230f..b0acba2 100644 --- a/src/tdm_capture.c +++ b/src/tdm_capture.c @@ -169,7 +169,7 @@ _tdm_capture_cb_done(tdm_capture *capture_module, tbm_surface_h buffer, void *us capture_done.base.sync = 0; capture_done.buffer = buffer; - ret = tdm_thread_cb_call(private_capture, &capture_done.base); + ret = tdm_thread_cb_call(private_capture, &capture_done.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); } diff --git a/src/tdm_output.c b/src/tdm_output.c index 652e262..bf353ce 100644 --- a/src/tdm_output.c +++ b/src/tdm_output.c @@ -269,7 +269,7 @@ _tdm_output_call_thread_cb_status(tdm_private_output *private_output, tdm_output output_status.base.sync = 1; output_status.status = status; - ret = tdm_thread_cb_call(private_output, &output_status.base); + ret = tdm_thread_cb_call(private_output, &output_status.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); return TDM_ERROR_NONE; @@ -289,7 +289,7 @@ _tdm_output_call_thread_cb_dpms(tdm_private_output *private_output, tdm_output_d output_dpms.base.sync = 0; output_dpms.dpms = dpms; - ret = tdm_thread_cb_call(private_output, &output_dpms.base); + ret = tdm_thread_cb_call(private_output, &output_dpms.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); return TDM_ERROR_NONE; @@ -811,7 +811,7 @@ _tdm_output_cb_vblank(tdm_output *output_backend, unsigned int sequence, if (tdm_debug_module & TDM_DEBUG_COMMIT) TDM_INFO("output(%d) wait_vblank: handler(%p)", vblank_handler->private_output->pipe, vblank_handler); - ret = tdm_thread_cb_call(vblank_handler->private_output, &output_vblank.base); + ret = tdm_thread_cb_call(vblank_handler->private_output, &output_vblank.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); } @@ -887,7 +887,7 @@ _tdm_output_cb_commit(tdm_output *output_backend, unsigned int sequence, output_commit.tv_sec = tv_sec; output_commit.tv_usec = tv_usec; - ret = tdm_thread_cb_call(private_output, &output_commit.base); + ret = tdm_thread_cb_call(private_output, &output_commit.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); } @@ -2164,7 +2164,7 @@ _need_validate_handler(int fd, tdm_event_loop_mask mask, void *user_data) ev.base.data = NULL; ev.base.sync = 0; - ret = tdm_thread_cb_call(private_output, &ev.base); + ret = tdm_thread_cb_call(private_output, &ev.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); TDM_INFO("tdm-thread: get a 'need to revalidate' event for the ouptut:%p.", private_output); diff --git a/src/tdm_pp.c b/src/tdm_pp.c index d9fcc31..ae5022e 100644 --- a/src/tdm_pp.c +++ b/src/tdm_pp.c @@ -173,7 +173,7 @@ _tdm_pp_cb_done(tdm_pp *pp_module, tbm_surface_h src, tbm_surface_h dst, void *u pp_done.src = src; pp_done.dst = dst; - ret = tdm_thread_cb_call(private_pp, &pp_done.base); + ret = tdm_thread_cb_call(private_pp, &pp_done.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); } diff --git a/src/tdm_thread.c b/src/tdm_thread.c index 6e891d7..29d996a 100644 --- a/src/tdm_thread.c +++ b/src/tdm_thread.c @@ -439,7 +439,8 @@ tdm_thread_handle_cb(tdm_private_loop *private_loop) case TDM_THREAD_CB_VBLANK_SW: case TDM_THREAD_CB_VBLANK_CREATE: case TDM_THREAD_CB_NEED_VALIDATE: - ret = tdm_thread_cb_call(NULL, base); + /* this event comes from other thread. so we don't need to propagate this to other thread */ + ret = tdm_thread_cb_call(NULL, base, 0); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); break; default: @@ -594,7 +595,7 @@ tdm_thread_cb_remove(void *object, tdm_thread_cb_type cb_type, void *cb_data, td * because a callback is added with cb_type and cb_data. */ INTERN tdm_error -tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) +tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base, unsigned int propagation) { tdm_private_display *private_display = tdm_display_get(); tdm_private_thread_cb *cb = NULL, *hh = NULL; @@ -603,7 +604,6 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) struct list_head *list, *other_list; struct list_head call_list; static pid_t waiting_tid = 0; - static tdm_thread_cb_type waiting_cb_type = TDM_THREAD_CB_NONE; tdm_error ret; TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); @@ -616,12 +616,15 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) assert(find_funcs[cb_base->type] != NULL); - /* handling only output-status as sync */ - if (cb_base->type == TDM_THREAD_CB_OUTPUT_STATUS) { - TDM_RETURN_VAL_IF_FAIL(cb_base->sync == 1, TDM_ERROR_INVALID_PARAMETER); - } else { - TDM_RETURN_VAL_IF_FAIL(cb_base->sync == 0, TDM_ERROR_INVALID_PARAMETER); - } + if (tdm_debug_module & TDM_DEBUG_THREAD) + TDM_INFO("'%s' thread_cb (sync:%d, propagation:%d) ------", + tdm_cb_type_str(cb_base->type), cb_base->sync, propagation); + + /* handling only output-status as sync. below logic can't handle two sync-type events */ + if (cb_base->type == TDM_THREAD_CB_OUTPUT_STATUS) + assert(cb_base->sync == 1); + else + assert(cb_base->sync == 0); if (!object) { object = find_funcs[cb_base->type](private_display, cb_base->object_stamp); @@ -665,9 +668,7 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) pthread_mutex_unlock(&cb_list_lock); - assert(LIST_IS_EMPTY(&call_list)); - - if (waiting_tid == 0 || waiting_cb_type != cb_base->type) { + if (propagation) { LIST_FOR_EACH_ENTRY_SAFE(cb, hh, other_list, link) { if (cb->object != object || cb->cb_type != cb_base->type || @@ -681,27 +682,34 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) if (!handler_in_other_thread) { if (keep_private_thread) { - if (cb_base->sync) { + if (cb_base->sync && waiting_tid != 0) { waiting_tid = 0; - waiting_cb_type = TDM_THREAD_CB_NONE; pthread_cond_signal(&keep_private_thread->event_cond); if (tdm_debug_module & TDM_DEBUG_THREAD) TDM_INFO("pthread broadcase"); } } if (tdm_debug_module & TDM_DEBUG_THREAD) - TDM_INFO("'%s' thread_cb done(sync:%d)", tdm_cb_type_str(cb_base->type), cb_base->sync); + TDM_INFO("'%s' thread_cb (sync:%d, propagation:%d) ------...", + tdm_cb_type_str(cb_base->type), cb_base->sync, propagation); return TDM_ERROR_NONE; } /* Once we reach here, it means that keep_private_thread is not NULL. * Just make the crash. Avoiding it is not going to help us. */ + assert(keep_private_thread != NULL); + ret = tdm_thread_send_cb(private_display->private_loop, cb_base); TDM_RETURN_VAL_IF_FAIL(ret == TDM_ERROR_NONE, TDM_ERROR_OPERATION_FAILED); /* waiting until all cb are done in another thread */ if (cb_base->sync) { + /* if waiting_tid is not 0, it means there are two sync-type events at the same time. + * and it would make deadlock issue. + */ + assert(waiting_tid == 0); + if (tdm_debug_module & TDM_DEBUG_THREAD) TDM_INFO("pthread wait"); @@ -710,14 +718,13 @@ tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base) */ tdm_mutex_locked = 0; waiting_tid = caller_tid; - waiting_cb_type = cb_base->type; - pthread_cond_wait(&keep_private_thread->event_cond, &private_display->lock); tdm_mutex_locked = 1; } if (tdm_debug_module & TDM_DEBUG_THREAD) - TDM_INFO("'%s' thread_cb done(sync:%d)", tdm_cb_type_str(cb_base->type), cb_base->sync); + TDM_INFO("'%s' thread_cb (sync:%d, propagation:%d) ------...", + tdm_cb_type_str(cb_base->type), cb_base->sync, propagation); return TDM_ERROR_NONE; } diff --git a/src/tdm_thread.h b/src/tdm_thread.h index 1a91933..57c4c6f 100644 --- a/src/tdm_thread.h +++ b/src/tdm_thread.h @@ -67,7 +67,7 @@ tdm_thread_cb_add(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_t void tdm_thread_cb_remove(void *object, tdm_thread_cb_type cb_type, void *cb_data, tdm_thread_cb func, void *user_data); tdm_error -tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base); +tdm_thread_cb_call(void *object, tdm_thread_cb_base *cb_base, unsigned int propagation); #ifdef __cplusplus diff --git a/src/tdm_vblank.c b/src/tdm_vblank.c index 263f825..cbedea7 100644 --- a/src/tdm_vblank.c +++ b/src/tdm_vblank.c @@ -601,7 +601,7 @@ _tdm_vblank_call_thread_cb(tdm_private_vblank *private_vblank) vblank_create.base.sync = 0; vblank_create.vblank_stamp = private_vblank->stamp; - ret = tdm_thread_cb_call(private_vblank->dpy, &vblank_create.base); + ret = tdm_thread_cb_call(private_vblank->dpy, &vblank_create.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); return TDM_ERROR_NONE; @@ -1410,7 +1410,7 @@ _tdm_vblank_cb_timeout_SW(void *user_data) vblank_sw.base.data = NULL; vblank_sw.base.sync = 0; - ret = tdm_thread_cb_call(private_vblank, &vblank_sw.base); + ret = tdm_thread_cb_call(private_vblank, &vblank_sw.base, 1); TDM_WARNING_IF_FAIL(ret == TDM_ERROR_NONE); return TDM_ERROR_NONE; -- 2.7.4