From 079e504c8b39826dbc840df05a3a41bef876ae72 Mon Sep 17 00:00:00 2001 From: Boram Park Date: Thu, 21 Apr 2016 15:43:47 +0900 Subject: [PATCH] fix deadlock and enhance lock/unlock to protect the backend module's data Change-Id: I4e057d3238779702af5e878be96ecb9c33573d10 --- src/tdm.c | 9 +++++++ src/tdm_backend.c | 20 +++++++------- src/tdm_buffer.c | 3 +++ src/tdm_capture.c | 29 ++++++++++---------- src/tdm_display.c | 75 ++++++++++++---------------------------------------- src/tdm_event_loop.c | 32 ++++++++++++++++++++++ src/tdm_helper.c | 25 ------------------ src/tdm_pp.c | 31 ++++++++++------------ src/tdm_private.h | 50 ++++++++++++++++++++++++++--------- src/tdm_server.c | 12 ++++----- src/tdm_thread.c | 36 ++++++++++++++++++++----- 11 files changed, 172 insertions(+), 150 deletions(-) diff --git a/src/tdm.c b/src/tdm.c index 24ee038..cc21a1f 100755 --- a/src/tdm.c +++ b/src/tdm.c @@ -42,6 +42,9 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include "tdm_private.h" #include "tdm_helper.h" +pthread_mutex_t tdm_mutex_check_lock = PTHREAD_MUTEX_INITIALIZER; +int tdm_mutex_locked; + static tdm_private_layer * _tdm_display_find_private_layer(tdm_private_output *private_output, tdm_layer *layer_backend) @@ -784,6 +787,8 @@ tdm_display_init(tdm_error *error) goto failed_mutex_init; } + _pthread_mutex_lock(&private_display->lock); + ret = tdm_event_loop_init(private_display); if (ret != TDM_ERROR_NONE) goto failed_event; @@ -821,6 +826,7 @@ tdm_display_init(tdm_error *error) if (error) *error = TDM_ERROR_NONE; + _pthread_mutex_unlock(&private_display->lock); _pthread_mutex_unlock(&gLock); return (tdm_display *)private_display; @@ -830,6 +836,7 @@ failed_update: failed_load: tdm_event_loop_deinit(private_display); failed_event: + _pthread_mutex_unlock(&private_display->lock); pthread_mutex_destroy(&private_display->lock); failed_mutex_init: free(private_display); @@ -881,5 +888,7 @@ tdm_display_deinit(tdm_display *dpy) tdm_debug_buffer = 0; _pthread_mutex_unlock(&gLock); + + TDM_INFO("done"); } diff --git a/src/tdm_backend.c b/src/tdm_backend.c index 9dfa2d1..d7cfc8d 100644 --- a/src/tdm_backend.c +++ b/src/tdm_backend.c @@ -69,6 +69,8 @@ tdm_backend_register_func_display(tdm_display *dpy, { tdm_backend_module *module; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + BACKEND_FUNC_ENTRY(); TDM_RETURN_VAL_IF_FAIL(func_display != NULL, TDM_ERROR_INVALID_PARAMETER); @@ -78,9 +80,7 @@ tdm_backend_register_func_display(tdm_display *dpy, if (_check_abi_version(module, 1, 1) < 0) return TDM_ERROR_BAD_MODULE; - _pthread_mutex_lock(&private_display->lock); private_display->func_display = *func_display; - _pthread_mutex_unlock(&private_display->lock); return TDM_ERROR_NONE; } @@ -90,6 +90,8 @@ tdm_backend_register_func_output(tdm_display *dpy, tdm_func_output *func_output) { tdm_backend_module *module; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + BACKEND_FUNC_ENTRY(); TDM_RETURN_VAL_IF_FAIL(func_output != NULL, TDM_ERROR_INVALID_PARAMETER); @@ -99,9 +101,7 @@ tdm_backend_register_func_output(tdm_display *dpy, tdm_func_output *func_output) if (_check_abi_version(module, 1, 1) < 0) return TDM_ERROR_BAD_MODULE; - _pthread_mutex_lock(&private_display->lock); private_display->func_output = *func_output; - _pthread_mutex_unlock(&private_display->lock); return TDM_ERROR_NONE; } @@ -111,6 +111,8 @@ tdm_backend_register_func_layer(tdm_display *dpy, tdm_func_layer *func_layer) { tdm_backend_module *module; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + BACKEND_FUNC_ENTRY(); TDM_RETURN_VAL_IF_FAIL(func_layer != NULL, TDM_ERROR_INVALID_PARAMETER); @@ -120,9 +122,7 @@ tdm_backend_register_func_layer(tdm_display *dpy, tdm_func_layer *func_layer) if (_check_abi_version(module, 1, 1) < 0) return TDM_ERROR_BAD_MODULE; - _pthread_mutex_lock(&private_display->lock); private_display->func_layer = *func_layer; - _pthread_mutex_unlock(&private_display->lock); return TDM_ERROR_NONE; } @@ -130,15 +130,15 @@ tdm_backend_register_func_layer(tdm_display *dpy, tdm_func_layer *func_layer) EXTERN tdm_error tdm_backend_register_func_pp(tdm_display *dpy, tdm_func_pp *func_pp) { + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + BACKEND_FUNC_ENTRY(); if (!func_pp) return TDM_ERROR_NONE; - _pthread_mutex_lock(&private_display->lock); private_display->capabilities |= TDM_DISPLAY_CAPABILITY_PP; private_display->func_pp = *func_pp; - _pthread_mutex_unlock(&private_display->lock); return TDM_ERROR_NONE; } @@ -147,15 +147,15 @@ EXTERN tdm_error tdm_backend_register_func_capture(tdm_display *dpy, tdm_func_capture *func_capture) { + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + BACKEND_FUNC_ENTRY(); if (!func_capture) return TDM_ERROR_NONE; - _pthread_mutex_lock(&private_display->lock); private_display->capabilities |= TDM_DISPLAY_CAPABILITY_CAPTURE; private_display->func_capture = *func_capture; - _pthread_mutex_unlock(&private_display->lock); return TDM_ERROR_NONE; } diff --git a/src/tdm_buffer.c b/src/tdm_buffer.c index 50a94be..a394fe7 100644 --- a/src/tdm_buffer.c +++ b/src/tdm_buffer.c @@ -196,6 +196,9 @@ tdm_buffer_unref_backend(tbm_surface_h buffer) return; } + if (!tdm_thread_in_display_thread(syscall(SYS_gettid))) + TDM_NEVER_GET_HERE(); + LIST_FOR_EACH_ENTRY_SAFE(func_info, next, &buf_info->release_funcs, link) { tbm_surface_internal_ref(buffer); func_info->release_func(buffer, func_info->user_data); diff --git a/src/tdm_capture.c b/src/tdm_capture.c index 411d209..1a73db4 100644 --- a/src/tdm_capture.c +++ b/src/tdm_capture.c @@ -82,8 +82,8 @@ tdm_capture_cb_done(tdm_capture *capture_backend, tbm_surface_h buffer, tdm_private_display *private_display = private_capture->private_display; tdm_buffer_info *buf_info; tbm_surface_h first_entry; - int lock_after_cb_done = 0; - int ret; + + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); if (private_capture->owner_tid != syscall(SYS_gettid)) { tdm_thread_cb_capture_done capture_done; @@ -114,18 +114,9 @@ tdm_capture_cb_done(tdm_capture *capture_backend, tbm_surface_h buffer, if ((buf_info = tdm_buffer_get_info(buffer))) LIST_DEL(&buf_info->link); - ret = pthread_mutex_trylock(&private_display->lock); - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - lock_after_cb_done = 1; - } - + _pthread_mutex_unlock(&private_display->lock); tdm_buffer_unref_backend(buffer); - - if (lock_after_cb_done) - _pthread_mutex_lock(&private_display->lock); + _pthread_mutex_lock(&private_display->lock); } INTERN tdm_private_capture * @@ -133,6 +124,8 @@ tdm_capture_find_stamp(tdm_private_display *private_display, unsigned long stamp { tdm_private_capture *private_capture = NULL; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), NULL); + LIST_FOR_EACH_ENTRY(private_capture, &private_display->capture_list, link) { if (private_capture->stamp == stamp) return private_capture; @@ -152,6 +145,8 @@ tdm_capture_create_output_internal(tdm_private_output *private_output, tdm_capture *capture_backend = NULL; tdm_error ret = TDM_ERROR_NONE; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), NULL); + if (!(private_display->capabilities & TDM_DISPLAY_CAPABILITY_CAPTURE)) { TDM_ERR("no capture capability"); if (error) @@ -221,6 +216,8 @@ tdm_capture_create_layer_internal(tdm_private_layer *private_layer, tdm_capture *capture_backend = NULL; tdm_error ret = TDM_ERROR_NONE; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), NULL); + if (!(private_display->capabilities & TDM_DISPLAY_CAPABILITY_CAPTURE)) { TDM_ERR("no capture capability"); if (error) @@ -272,6 +269,8 @@ tdm_capture_destroy_internal(tdm_private_capture *private_capture) tdm_func_capture *func_capture; tdm_buffer_info *b = NULL, *bb = NULL; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); + if (!private_capture) return; @@ -282,7 +281,7 @@ tdm_capture_destroy_internal(tdm_private_capture *private_capture) func_capture->capture_destroy(private_capture->capture_backend); if (!LIST_IS_EMPTY(&private_capture->pending_buffer_list)) { - TDM_ERR("capture(%p) not finished:", private_capture); + TDM_WRN("capture(%p) not finished:", private_capture); tdm_buffer_list_dump(&private_capture->pending_buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_capture->pending_buffer_list, link) { @@ -294,7 +293,7 @@ tdm_capture_destroy_internal(tdm_private_capture *private_capture) } if (!LIST_IS_EMPTY(&private_capture->buffer_list)) { - TDM_ERR("capture(%p) not finished:", private_capture); + TDM_WRN("capture(%p) not finished:", private_capture); tdm_buffer_list_dump(&private_capture->buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_capture->buffer_list, link) { diff --git a/src/tdm_display.c b/src/tdm_display.c index a7a1327..4a2d25e 100644 --- a/src/tdm_display.c +++ b/src/tdm_display.c @@ -341,11 +341,8 @@ tdm_display_get_fd(tdm_display *dpy, int *fd) _pthread_mutex_lock(&private_display->lock); - if (private_display->private_loop->private_thread) { - _pthread_mutex_unlock(&private_display->lock); + if (tdm_thread_is_running()) *fd = tdm_thread_get_fd(private_display->private_loop); - _pthread_mutex_lock(&private_display->lock); - } else *fd = tdm_event_loop_get_fd(private_display); @@ -384,18 +381,11 @@ tdm_display_handle_events(tdm_display *dpy) if (tdm_debug_thread) TDM_INFO("fd(%d) polling out", fd); - _pthread_mutex_lock(&private_display->lock); - - if (private_display->private_loop->private_thread) { - _pthread_mutex_unlock(&private_display->lock); + if (tdm_thread_is_running()) ret = tdm_thread_handle_cb(private_display->private_loop); - _pthread_mutex_lock(&private_display->lock); - } else ret = tdm_event_loop_dispatch(private_display); - _pthread_mutex_unlock(&private_display->lock); - return ret; } @@ -458,15 +448,13 @@ tdm_output_cb_status(tdm_output *output_backend, tdm_output_conn_status status, tdm_private_display *private_display; tdm_private_output *private_output = user_data; tdm_value value; - int lock_after_cb_done = 0; - int ret; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); TDM_RETURN_IF_FAIL(private_output); private_display = private_output->private_display; - if (!tdm_thread_in_display_thread(private_display->private_loop, - syscall(SYS_gettid))) { + if (!tdm_thread_in_display_thread(syscall(SYS_gettid))) { tdm_thread_cb_output_status output_status; tdm_error ret; @@ -488,22 +476,11 @@ tdm_output_cb_status(tdm_output *output_backend, tdm_output_conn_status status, return; } - ret = pthread_mutex_trylock(&private_display->lock); - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - lock_after_cb_done = 1; - } - value.u32 = status; tdm_output_call_change_handler_internal(private_output, &private_output->change_handler_list_main, TDM_OUTPUT_CHANGE_CONNECTION, value); - - if (lock_after_cb_done) - _pthread_mutex_lock(&private_display->lock); } EXTERN tdm_error @@ -536,8 +513,7 @@ tdm_output_add_change_handler(tdm_output *output, change_handler->user_data = user_data; change_handler->owner_tid = syscall(SYS_gettid); - if (!tdm_thread_in_display_thread(private_display->private_loop, - change_handler->owner_tid)) + if (!tdm_thread_in_display_thread(change_handler->owner_tid)) LIST_ADD(&change_handler->link, &private_output->change_handler_list_sub); else LIST_ADD(&change_handler->link, &private_output->change_handler_list_main); @@ -826,6 +802,7 @@ tdm_output_cb_vblank(tdm_output *output_backend, unsigned int sequence, tdm_private_vblank_handler *vblank_handler = user_data; tdm_private_display *private_display; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); TDM_RETURN_IF_FAIL(vblank_handler); private_display = vblank_handler->private_output->private_display; @@ -852,22 +829,10 @@ tdm_output_cb_vblank(tdm_output *output_backend, unsigned int sequence, TDM_NEVER_GET_HERE(); if (vblank_handler->func) { - int lock_after_cb_done = 0; - int ret; - - ret = pthread_mutex_trylock(&private_display->lock); - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - lock_after_cb_done = 1; - } - + _pthread_mutex_unlock(&private_display->lock); vblank_handler->func(vblank_handler->private_output, sequence, tv_sec, tv_usec, vblank_handler->user_data); - - if (lock_after_cb_done) - _pthread_mutex_lock(&private_display->lock); + _pthread_mutex_lock(&private_display->lock); } LIST_DEL(&vblank_handler->link); @@ -882,9 +847,8 @@ tdm_output_cb_commit(tdm_output *output_backend, unsigned int sequence, tdm_private_display *private_display; tdm_private_output *private_output; tdm_private_layer *private_layer = NULL; - int lock_after_cb_done = 0; - int ret; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); TDM_RETURN_IF_FAIL(commit_handler); private_output = commit_handler->private_output; @@ -908,20 +872,14 @@ tdm_output_cb_commit(tdm_output *output_backend, unsigned int sequence, return; } - ret = pthread_mutex_trylock(&private_display->lock); - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - lock_after_cb_done = 1; - } - LIST_FOR_EACH_ENTRY(private_layer, &private_output->layer_list, link) { if (!private_layer->waiting_buffer) continue; if (private_layer->showing_buffer) { + _pthread_mutex_unlock(&private_display->lock); tdm_buffer_unref_backend(private_layer->showing_buffer); + _pthread_mutex_lock(&private_display->lock); if (private_layer->buffer_queue) { tbm_surface_queue_release(private_layer->buffer_queue, @@ -939,12 +897,11 @@ tdm_output_cb_commit(tdm_output *output_backend, unsigned int sequence, } if (commit_handler->func) { + _pthread_mutex_unlock(&private_display->lock); commit_handler->func(private_output, sequence, tv_sec, tv_usec, commit_handler->user_data); - } - - if (lock_after_cb_done) _pthread_mutex_lock(&private_display->lock); + } LIST_DEL(&commit_handler->link); free(commit_handler); @@ -1213,11 +1170,11 @@ tdm_output_call_change_handler_internal(tdm_private_output *private_output, tdm_private_display *private_display; tdm_private_change_handler *change_handler; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); TDM_RETURN_IF_FAIL(private_output); private_display = private_output->private_display; - if (!tdm_thread_in_display_thread(private_display->private_loop, - syscall(SYS_gettid))) { + if (!tdm_thread_in_display_thread(syscall(SYS_gettid))) { if (type & TDM_OUTPUT_CHANGE_CONNECTION) TDM_INFO("output(%d) changed: %s (%d)", private_output->pipe, status_str(value.u32), value.u32); @@ -1233,8 +1190,10 @@ tdm_output_call_change_handler_internal(tdm_private_output *private_output, if (change_handler->owner_tid != syscall(SYS_gettid)) TDM_NEVER_GET_HERE(); + _pthread_mutex_unlock(&private_display->lock); change_handler->func(private_output, type, value, change_handler->user_data); + _pthread_mutex_lock(&private_display->lock); } } diff --git a/src/tdm_event_loop.c b/src/tdm_event_loop.c index e0791ab..a9da07c 100644 --- a/src/tdm_event_loop.c +++ b/src/tdm_event_loop.c @@ -72,6 +72,7 @@ _tdm_event_loop_main_fd_handler(int fd, tdm_event_loop_mask mask, void *user_dat tdm_func_display *func_display; tdm_error ret; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(private_display != NULL, TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(private_display->private_loop != NULL, TDM_ERROR_OPERATION_FAILED); @@ -95,6 +96,8 @@ tdm_event_loop_init(tdm_private_display *private_display) tdm_private_loop *private_loop; tdm_error ret; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); + if (private_display->private_loop) return TDM_ERROR_NONE; @@ -142,6 +145,8 @@ tdm_event_loop_init(tdm_private_display *private_display) INTERN void tdm_event_loop_deinit(tdm_private_display *private_display) { + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); + if (!private_display->private_loop) return; @@ -166,6 +171,7 @@ tdm_event_loop_create_backend_source(tdm_private_display *private_display) tdm_error ret; int fd = -1; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); TDM_RETURN_IF_FAIL(private_loop != NULL); func_display = &private_display->func_display; @@ -205,6 +211,8 @@ tdm_event_loop_get_fd(tdm_private_display *private_display) { tdm_private_loop *private_loop = private_display->private_loop; + /* DON'T check TDM_MUTEX_IS_LOCKED here */ + TDM_RETURN_VAL_IF_FAIL(private_loop->wl_loop != NULL, -1); return wl_event_loop_get_fd(private_loop->wl_loop); @@ -215,6 +223,8 @@ tdm_event_loop_dispatch(tdm_private_display *private_display) { tdm_private_loop *private_loop = private_display->private_loop; + /* DON'T check TDM_MUTEX_IS_LOCKED here */ + TDM_RETURN_VAL_IF_FAIL(private_loop->wl_loop != NULL, TDM_ERROR_OPERATION_FAILED); if (tdm_debug_thread) @@ -236,6 +246,8 @@ tdm_event_loop_flush(tdm_private_display *private_display) { tdm_private_loop *private_loop = private_display->private_loop; + /* DON'T check TDM_MUTEX_IS_LOCKED here */ + TDM_RETURN_IF_FAIL(private_loop->wl_display != NULL); wl_display_flush_clients(private_loop->wl_display); @@ -245,11 +257,16 @@ static int _tdm_event_loop_fd_func(int fd, uint32_t wl_mask, void *data) { tdm_event_loop_source_fd *fd_source = (tdm_event_loop_source_fd*)data; + tdm_private_display *private_display; tdm_event_loop_mask mask = 0; + /* DON'T check TDM_MUTEX_IS_LOCKED here */ + TDM_RETURN_VAL_IF_FAIL(fd_source, 1); TDM_RETURN_VAL_IF_FAIL(fd_source->func, 1); + private_display = fd_source->private_display; + if (wl_mask & WL_EVENT_READABLE) mask |= TDM_EVENT_LOOP_READABLE; if (wl_mask & WL_EVENT_WRITABLE) @@ -259,7 +276,9 @@ _tdm_event_loop_fd_func(int fd, uint32_t wl_mask, void *data) if (wl_mask & WL_EVENT_ERROR) mask |= TDM_EVENT_LOOP_ERROR; + _pthread_mutex_lock(&private_display->lock); fd_source->func(fd, mask, fd_source->user_data); + _pthread_mutex_unlock(&private_display->lock); return 1; } @@ -275,6 +294,7 @@ tdm_event_loop_add_fd_handler(tdm_display *dpy, int fd, tdm_event_loop_mask mask uint32_t wl_mask = 0; tdm_error ret; + TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED, NULL); TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(dpy, TDM_ERROR_INVALID_PARAMETER, NULL); TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(fd >= 0, TDM_ERROR_INVALID_PARAMETER, NULL); TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(func, TDM_ERROR_INVALID_PARAMETER, NULL); @@ -318,6 +338,7 @@ tdm_event_loop_source_fd_update(tdm_event_loop_source *source, tdm_event_loop_ma tdm_event_loop_source_fd *fd_source = source; uint32_t wl_mask = 0; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(fd_source, TDM_ERROR_INVALID_PARAMETER); if (mask & TDM_EVENT_LOOP_READABLE) @@ -337,11 +358,18 @@ static int _tdm_event_loop_timer_func(void *data) { tdm_event_loop_source_timer *timer_source = (tdm_event_loop_source_timer*)data; + tdm_private_display *private_display; + + /* DON'T check TDM_MUTEX_IS_LOCKED here */ TDM_RETURN_VAL_IF_FAIL(timer_source, 1); TDM_RETURN_VAL_IF_FAIL(timer_source->func, 1); + private_display = timer_source->private_display; + + _pthread_mutex_lock(&private_display->lock); timer_source->func(timer_source->user_data); + _pthread_mutex_unlock(&private_display->lock); return 1; } @@ -355,6 +383,7 @@ tdm_event_loop_add_timer_handler(tdm_display *dpy, tdm_event_loop_timer_handler tdm_event_loop_source_timer *timer_source; tdm_error ret; + TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED, NULL); TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(dpy, TDM_ERROR_INVALID_PARAMETER, NULL); TDM_RETURN_VAL_IF_FAIL_WITH_ERROR(func, TDM_ERROR_INVALID_PARAMETER, NULL); @@ -391,6 +420,7 @@ tdm_event_loop_source_timer_update(tdm_event_loop_source *source, int ms_delay) { tdm_event_loop_source_timer *timer_source = source; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(timer_source, TDM_ERROR_INVALID_PARAMETER); if (wl_event_source_timer_update(timer_source->base.wl_source, ms_delay) < 0) { @@ -406,6 +436,8 @@ tdm_event_loop_source_remove(tdm_event_loop_source *source) { tdm_event_loop_source_base *base = (tdm_event_loop_source_base*)source; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); + if (!base) return; diff --git a/src/tdm_helper.c b/src/tdm_helper.c index 03d13cb..4e6c910 100644 --- a/src/tdm_helper.c +++ b/src/tdm_helper.c @@ -19,31 +19,6 @@ static const char *dump_prefix[2] = {"png", "yuv"}; static int *tdm_helper_dump_count; static char *tdm_helper_dump_path; -INTERN int -tdm_helper_unlock_in_cb(tdm_private_display *private_display) -{ - int ret = pthread_mutex_trylock(&private_display->lock); - int need_lock = 0; - - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - need_lock = 1; - } - - return need_lock; -} - -INTERN void -tdm_helper_lock_in_cb(tdm_private_display *private_display, int need_lock) -{ - if (!need_lock) - return; - - _pthread_mutex_lock(&private_display->lock); -} - INTERN unsigned long tdm_helper_get_time_in_millis(void) { diff --git a/src/tdm_pp.c b/src/tdm_pp.c index 603911e..0b56073 100644 --- a/src/tdm_pp.c +++ b/src/tdm_pp.c @@ -96,8 +96,8 @@ tdm_pp_cb_done(tdm_pp *pp_backend, tbm_surface_h src, tbm_surface_h dst, tdm_private_display *private_display = private_pp->private_display; tdm_buffer_info *buf_info; tbm_surface_h first_entry; - int lock_after_cb_done = 0; - int ret; + + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); if (private_pp->owner_tid != syscall(SYS_gettid)) { tdm_thread_cb_pp_done pp_done; @@ -136,19 +136,10 @@ tdm_pp_cb_done(tdm_pp *pp_backend, tbm_surface_h src, tbm_surface_h dst, if ((buf_info = tdm_buffer_get_info(dst))) LIST_DEL(&buf_info->link); - ret = pthread_mutex_trylock(&private_display->lock); - if (ret == 0) - _pthread_mutex_unlock(&private_display->lock); - else if (ret == EBUSY) { - _pthread_mutex_unlock(&private_display->lock); - lock_after_cb_done = 1; - } - + _pthread_mutex_unlock(&private_display->lock); tdm_buffer_unref_backend(src); tdm_buffer_unref_backend(dst); - - if (lock_after_cb_done) - _pthread_mutex_lock(&private_display->lock); + _pthread_mutex_lock(&private_display->lock); } INTERN tdm_private_pp * @@ -156,6 +147,8 @@ tdm_pp_find_stamp(tdm_private_display *private_display, unsigned long stamp) { tdm_private_pp *private_pp = NULL; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), NULL); + LIST_FOR_EACH_ENTRY(private_pp, &private_display->pp_list, link) { if (private_pp->stamp == stamp) return private_pp; @@ -173,6 +166,8 @@ tdm_pp_create_internal(tdm_private_display *private_display, tdm_error *error) tdm_pp *pp_backend = NULL; tdm_error ret = TDM_ERROR_NONE; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), NULL); + func_display = &private_display->func_display; func_pp = &private_display->func_pp; @@ -235,6 +230,8 @@ tdm_pp_destroy_internal(tdm_private_pp *private_pp) tdm_func_pp *func_pp; tdm_buffer_info *b = NULL, *bb = NULL; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); + if (!private_pp) return; @@ -246,7 +243,7 @@ tdm_pp_destroy_internal(tdm_private_pp *private_pp) func_pp->pp_destroy(private_pp->pp_backend); if (!LIST_IS_EMPTY(&private_pp->src_pending_buffer_list)) { - TDM_ERR("pp(%p) not finished:", private_pp); + TDM_WRN("pp(%p) not finished:", private_pp); tdm_buffer_list_dump(&private_pp->src_pending_buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_pp->src_pending_buffer_list, link) { @@ -258,7 +255,7 @@ tdm_pp_destroy_internal(tdm_private_pp *private_pp) } if (!LIST_IS_EMPTY(&private_pp->dst_pending_buffer_list)) { - TDM_ERR("pp(%p) not finished:", private_pp); + TDM_WRN("pp(%p) not finished:", private_pp); tdm_buffer_list_dump(&private_pp->dst_pending_buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_pp->dst_pending_buffer_list, link) { @@ -270,7 +267,7 @@ tdm_pp_destroy_internal(tdm_private_pp *private_pp) } if (!LIST_IS_EMPTY(&private_pp->src_buffer_list)) { - TDM_ERR("pp(%p) not finished:", private_pp); + TDM_WRN("pp(%p) not finished:", private_pp); tdm_buffer_list_dump(&private_pp->src_buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_pp->src_buffer_list, link) { @@ -282,7 +279,7 @@ tdm_pp_destroy_internal(tdm_private_pp *private_pp) } if (!LIST_IS_EMPTY(&private_pp->dst_buffer_list)) { - TDM_ERR("pp(%p) not finished:", private_pp); + TDM_WRN("pp(%p) not finished:", private_pp); tdm_buffer_list_dump(&private_pp->dst_buffer_list); LIST_FOR_EACH_ENTRY_SAFE(b, bb, &private_pp->dst_buffer_list, link) { diff --git a/src/tdm_private.h b/src/tdm_private.h index aa19633..7bc3acb 100644 --- a/src/tdm_private.h +++ b/src/tdm_private.h @@ -229,10 +229,10 @@ struct _tdm_private_capture { }; /* CAUTION: - * Note that this structure is not thread-safe. If there is no TDM thread, all - * TDM resources are protected by private_display's lock. If there is a TDM - * thread, this struct will be used only in a TDM thread. So, we don't need to - * protect this structure. + * Note that we don't need to (un)lock mutex to use this structure. If there is + * no TDM thread, all TDM resources are protected by private_display's mutex. + * If there is a TDM thread, this struct will be used only in a TDM thread. + * So, we don't need to protect this structure by mutex. Not thread-safe. */ struct _tdm_private_loop { /* TDM uses wl_event_loop to handle various event sources including the TDM @@ -247,7 +247,7 @@ struct _tdm_private_loop { /* In event loop, all resources are accessed by this dpy. * CAUTION: * - DO NOT include other private structure in this structure because this - * struct is not thread-safe. + * struct is not protected by mutex. */ tdm_display *dpy; @@ -432,7 +432,9 @@ tdm_thread_send_cb(tdm_private_loop *private_loop, tdm_thread_cb_base *base); tdm_error tdm_thread_handle_cb(tdm_private_loop *private_loop); int -tdm_thread_in_display_thread(tdm_private_loop *private_loop, pid_t tid); +tdm_thread_in_display_thread(pid_t tid); +int +tdm_thread_is_running(void); tdm_error tdm_server_init(tdm_private_loop *private_loop); @@ -441,10 +443,9 @@ tdm_server_deinit(tdm_private_loop *private_loop); unsigned long tdm_helper_get_time_in_millis(void); -int -tdm_helper_unlock_in_cb(tdm_private_display *private_display); -void -tdm_helper_lock_in_cb(tdm_private_display *private_display, int need_lock); + +extern pthread_mutex_t tdm_mutex_check_lock; +extern int tdm_mutex_locked; int tdm_helper_get_dump_count(void); @@ -452,9 +453,34 @@ char * tdm_helper_get_dump_path(void); #define _pthread_mutex_lock(l) \ - do {if (tdm_debug_mutex) TDM_INFO("mutex lock"); pthread_mutex_lock(l);} while (0) + do { \ + if (tdm_debug_mutex) \ + TDM_INFO("mutex lock"); \ + pthread_mutex_lock(l); \ + pthread_mutex_lock(&tdm_mutex_check_lock); \ + tdm_mutex_locked = 1; \ + pthread_mutex_unlock(&tdm_mutex_check_lock); \ + } while (0) + #define _pthread_mutex_unlock(l) \ - do {if (tdm_debug_mutex) TDM_INFO("mutex unlock"); pthread_mutex_unlock(l);} while (0) + do { \ + if (tdm_debug_mutex) \ + TDM_INFO("mutex unlock"); \ + pthread_mutex_lock(&tdm_mutex_check_lock); \ + tdm_mutex_locked = 0; \ + pthread_mutex_unlock(&tdm_mutex_check_lock); \ + pthread_mutex_unlock(l); \ + } while (0) + +//#define TDM_MUTEX_IS_LOCKED() (tdm_mutex_locked == 1) +static inline int TDM_MUTEX_IS_LOCKED(void) +{ + int ret; + pthread_mutex_lock(&tdm_mutex_check_lock); + ret = (tdm_mutex_locked == 1); + pthread_mutex_unlock(&tdm_mutex_check_lock); + return ret; +} #ifdef __cplusplus } diff --git a/src/tdm_server.c b/src/tdm_server.c index c824905..142c1a3 100644 --- a/src/tdm_server.c +++ b/src/tdm_server.c @@ -43,7 +43,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include "tdm-server-protocol.h" /* CAUTION: - * - tdm server doesn't care about multi-thread. + * - tdm server doesn't care about thread things. * - DO NOT use the TDM internal functions here. */ @@ -57,7 +57,7 @@ typedef struct _tdm_server_vblank_info { tdm_private_server *private_server; } tdm_server_vblank_info; -static tdm_private_loop *keep_private_loop; +static tdm_private_server *keep_private_server; static void _tdm_server_cb_output_vblank(tdm_output *output, unsigned int sequence, @@ -67,10 +67,10 @@ _tdm_server_cb_output_vblank(tdm_output *output, unsigned int sequence, tdm_server_vblank_info *vblank_info = (tdm_server_vblank_info*)user_data; tdm_server_vblank_info *found; - if (!keep_private_loop || !keep_private_loop->private_server) + if (!keep_private_server) return; - LIST_FIND_ITEM(vblank_info, &keep_private_loop->private_server->vblank_list, + LIST_FIND_ITEM(vblank_info, &keep_private_server->vblank_list, tdm_server_vblank_info, link, found); if (!found) { TDM_DBG("vblank_info(%p) is destroyed", vblank_info); @@ -227,7 +227,7 @@ tdm_server_init(tdm_private_loop *private_loop) } private_loop->private_server = private_server; - keep_private_loop = private_loop; + keep_private_server = private_server; return TDM_ERROR_NONE; } @@ -249,5 +249,5 @@ tdm_server_deinit(tdm_private_loop *private_loop) free(private_server); private_loop->private_server = NULL; - keep_private_loop = NULL; + keep_private_server = NULL; } diff --git a/src/tdm_thread.c b/src/tdm_thread.c index fd9a5ad..53ab893 100644 --- a/src/tdm_thread.c +++ b/src/tdm_thread.c @@ -43,6 +43,8 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include "tdm_private.h" #include "tdm_list.h" +static tdm_private_thread *keep_private_thread; + struct _tdm_private_thread { tdm_private_loop *private_loop; @@ -124,6 +126,7 @@ tdm_thread_init(tdm_private_loop *private_loop) tdm_private_thread *private_thread; const char *thread; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(private_loop->dpy, TDM_ERROR_OPERATION_FAILED); private_display = private_loop->dpy; @@ -159,6 +162,8 @@ tdm_thread_init(tdm_private_loop *private_loop) pthread_create(&private_thread->event_thread, NULL, _tdm_thread_main, private_thread); + keep_private_thread = private_thread; + TDM_INFO("using a TDM event thread. pipe(%d,%d)", private_thread->pipe[0], private_thread->pipe[1]); @@ -168,6 +173,8 @@ tdm_thread_init(tdm_private_loop *private_loop) INTERN void tdm_thread_deinit(tdm_private_loop *private_loop) { + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); + if (!private_loop->private_thread) return; @@ -181,6 +188,7 @@ tdm_thread_deinit(tdm_private_loop *private_loop) free(private_loop->private_thread); private_loop->private_thread = NULL; + keep_private_thread = NULL; TDM_INFO("Finish a TDM event thread"); } @@ -190,6 +198,7 @@ tdm_thread_get_fd(tdm_private_loop *private_loop) { tdm_private_thread *private_thread; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(private_loop, -1); TDM_RETURN_VAL_IF_FAIL(private_loop->private_thread, -1); @@ -204,6 +213,7 @@ tdm_thread_send_cb(tdm_private_loop *private_loop, tdm_thread_cb_base *base) tdm_private_thread *private_thread; ssize_t len; + TDM_RETURN_VAL_IF_FAIL(TDM_MUTEX_IS_LOCKED(), TDM_ERROR_OPERATION_FAILED); TDM_RETURN_VAL_IF_FAIL(base, TDM_ERROR_INVALID_PARAMETER); TDM_RETURN_VAL_IF_FAIL(private_loop, TDM_ERROR_INVALID_PARAMETER); TDM_RETURN_VAL_IF_FAIL(private_loop->private_thread, TDM_ERROR_INVALID_PARAMETER); @@ -226,15 +236,19 @@ tdm_thread_send_cb(tdm_private_loop *private_loop, tdm_thread_cb_base *base) INTERN tdm_error tdm_thread_handle_cb(tdm_private_loop *private_loop) { + tdm_private_display *private_display; tdm_private_thread *private_thread; tdm_thread_cb_base *base; char buffer[1024]; int len, i; + /* DON'T check TDM_MUTEX_IS_LOCKED here */ + TDM_RETURN_VAL_IF_FAIL(private_loop, TDM_ERROR_INVALID_PARAMETER); TDM_RETURN_VAL_IF_FAIL(private_loop->private_thread, TDM_ERROR_INVALID_PARAMETER); private_thread = private_loop->private_thread; + private_display = private_loop->dpy; len = read(private_thread->pipe[0], buffer, sizeof buffer); @@ -249,6 +263,8 @@ tdm_thread_handle_cb(tdm_private_loop *private_loop) return TDM_ERROR_OPERATION_FAILED; } + _pthread_mutex_lock(&private_display->lock); + i = 0; while (i < len) { base = (tdm_thread_cb_base*)&buffer[i]; @@ -326,22 +342,28 @@ tdm_thread_handle_cb(tdm_private_loop *private_loop) i += base->length; } + _pthread_mutex_unlock(&private_display->lock); + tdm_event_loop_flush(private_loop->dpy); return TDM_ERROR_NONE; } INTERN int -tdm_thread_in_display_thread(tdm_private_loop *private_loop, pid_t tid) +tdm_thread_in_display_thread(pid_t tid) { - tdm_private_thread *private_thread; + if (!keep_private_thread) + return 1; - TDM_RETURN_VAL_IF_FAIL(private_loop, 1); + /* DON'T check TDM_MUTEX_IS_LOCKED here */ - if (!private_loop->private_thread) - return 1; + return (keep_private_thread->display_tid == tid) ? 1 : 0; +} - private_thread = private_loop->private_thread; +INTERN int +tdm_thread_is_running(void) +{ + /* DON'T check TDM_MUTEX_IS_LOCKED here */ - return (private_thread->display_tid == tid) ? 1 : 0; + return (keep_private_thread) ? 1 : 0; } -- 2.7.4