From 85c44f9245e5b6d9f157c9966f49e12c594efebe Mon Sep 17 00:00:00 2001 From: Jamiras <32680403+Jamiras@users.noreply.github.com> Date: Tue, 23 Aug 2022 07:11:11 -0600 Subject: [PATCH] more thread-awareness in task callbacks (#14337) * more thread-awareness in task callbacks --- gfx/gfx_widgets.c | 25 +++++++++++-------------- gfx/gfx_widgets.h | 2 -- libretro-common/queues/task_queue.c | 13 +++++++++++-- tasks/task_core_updater.c | 27 +++++++++++---------------- tasks/task_decompress.c | 2 +- 5 files changed, 34 insertions(+), 35 deletions(-) diff --git a/gfx/gfx_widgets.c b/gfx/gfx_widgets.c index ab4ee714e8..9f76075430 100644 --- a/gfx/gfx_widgets.c +++ b/gfx/gfx_widgets.c @@ -215,9 +215,6 @@ void gfx_widgets_msg_queue_push( msg_widget = (disp_widget_msg_t*)malloc(sizeof(*msg_widget)); - if (task) - title = task->title; - msg_widget->msg = NULL; msg_widget->msg_new = NULL; msg_widget->msg_transition_animation = 0.0f; @@ -237,7 +234,6 @@ void gfx_widgets_msg_queue_push( msg_widget->expiration_timer_started = false; msg_widget->task_ptr = task; - msg_widget->task_title_ptr = NULL; msg_widget->task_count = 0; msg_widget->task_progress = 0; @@ -262,7 +258,7 @@ void gfx_widgets_msg_queue_push( if (task) { - msg_widget->msg = strdup(title); + title = msg_widget->msg = strdup(task->title); msg_widget->msg_new = strdup(title); msg_widget->msg_len = (unsigned)strlen(title); @@ -271,7 +267,6 @@ void gfx_widgets_msg_queue_push( msg_widget->task_finished = task->finished; msg_widget->task_progress = task->progress; msg_widget->task_ident = task->ident; - msg_widget->task_title_ptr = task->title; msg_widget->task_count = 1; msg_widget->unfolded = true; @@ -351,12 +346,7 @@ void gfx_widgets_msg_queue_push( if (!string_is_equal(task->title, msg_widget->msg_new)) { - unsigned len = (unsigned)strlen(task->title); - unsigned new_width = font_driver_get_message_width( - p_dispwidget->gfx_widget_fonts.msg_queue.font, - task->title, - len, - 1); + unsigned len, new_width; if (msg_widget->msg_new) { @@ -364,9 +354,16 @@ void gfx_widgets_msg_queue_push( msg_widget->msg_new = NULL; } - msg_widget->msg_new = strdup(task->title); + title = msg_widget->msg_new = strdup(task->title); + + len = (unsigned)strlen(title); + new_width = font_driver_get_message_width( + p_dispwidget->gfx_widget_fonts.msg_queue.font, + title, + len, + 1); + msg_widget->msg_len = len; - msg_widget->task_title_ptr = task->title; msg_widget->msg_transition_animation = 0; if (!task->alternative_look) diff --git a/gfx/gfx_widgets.h b/gfx/gfx_widgets.h index cddef240f4..5b3389c076 100644 --- a/gfx/gfx_widgets.h +++ b/gfx/gfx_widgets.h @@ -115,8 +115,6 @@ typedef struct disp_widget_msg char *msg; char *msg_new; retro_task_t *task_ptr; - /* Used to detect title change */ - char *task_title_ptr; uint32_t task_ident; unsigned msg_len; diff --git a/libretro-common/queues/task_queue.c b/libretro-common/queues/task_queue.c index 1e7ecde401..137e4adc4b 100644 --- a/libretro-common/queues/task_queue.c +++ b/libretro-common/queues/task_queue.c @@ -91,6 +91,13 @@ static void task_queue_msg_push(retro_task_t *task, static void task_queue_push_progress(retro_task_t *task) { +#ifdef HAVE_THREADS + /* msg_push callback interacts directly with the task properties (particularly title). + * make sure another thread doesn't modify them while rendering + */ + slock_lock(property_lock); +#endif + if (task->title && !task->mute) { if (task->finished) @@ -113,6 +120,10 @@ static void task_queue_push_progress(retro_task_t *task) if (task->progress_cb) task->progress_cb(task); } + +#ifdef HAVE_THREADS + slock_unlock(property_lock); +#endif } static void task_queue_put(task_queue_t *queue, retro_task_t *task) @@ -392,7 +403,6 @@ static void retro_task_threaded_gather(void) { retro_task_t *task = NULL; - slock_lock(property_lock); slock_lock(running_lock); for (task = tasks_running.front; task; task = task->next) task_queue_push_progress(task); @@ -401,7 +411,6 @@ static void retro_task_threaded_gather(void) slock_lock(finished_lock); retro_task_internal_gather(); slock_unlock(finished_lock); - slock_unlock(property_lock); } static void retro_task_threaded_wait(retro_task_condition_fn_t cond, void* data) diff --git a/tasks/task_core_updater.c b/tasks/task_core_updater.c index 9dcb52ff03..cf800db181 100644 --- a/tasks/task_core_updater.c +++ b/tasks/task_core_updater.c @@ -576,14 +576,8 @@ void cb_http_task_core_updater_download( if (!(download_handle = (core_updater_download_handle_t*)transf->user_data)) goto finish; - /* Update download_handle task status - * NOTE: We set decompress_task_complete = true - * here to prevent any lock-ups in the event - * of errors (or lack of decompression support). - * decompress_task_complete will be set false - * if/when we actually call task_push_decompress() */ + /* Update download_handle task status */ download_handle->http_task_complete = true; - download_handle->decompress_task_complete = true; /* Create output directory, if required */ strlcpy(output_dir, transf->path, sizeof(output_dir)); @@ -634,8 +628,6 @@ void cb_http_task_core_updater_download( err = msg_hash_to_str(MSG_DECOMPRESSION_FAILED); goto finish; } - - download_handle->decompress_task_complete = false; } #endif @@ -647,6 +639,10 @@ finish: if (transf) free(transf); + + /* if no decompress task was queued, mark it as completed */ + if (!download_handle->decompress_task) + download_handle->decompress_task_complete = true; } static void free_core_updater_download_handle(core_updater_download_handle_t *download_handle) @@ -893,13 +889,12 @@ static void task_core_updater_download_handler(retro_task_t *task) case CORE_UPDATER_DOWNLOAD_WAIT_DECOMPRESS: { /* If decompression task is NULL, then it either - * finished or an error occurred - in either case, - * just move on to the next state */ - if (!download_handle->decompress_task) - download_handle->decompress_task_complete = true; - /* Otherwise, check if decompression task is still - * running */ - else if (!download_handle->decompress_task_finished) + * hasn't been queued by the download task yet, + * or an error occurred. The latter should set + * the decompress_task_complete flag and we'll + * continue to the next state */ + if (download_handle->decompress_task && + !download_handle->decompress_task_finished) { download_handle->decompress_task_finished = task_get_finished(download_handle->decompress_task); diff --git a/tasks/task_decompress.c b/tasks/task_decompress.c index 78e0a90f1f..2d9aaa67b0 100644 --- a/tasks/task_decompress.c +++ b/tasks/task_decompress.c @@ -290,7 +290,7 @@ void *task_push_decompress( if (!(s = (decompress_state_t*)calloc(1, sizeof(*s)))) return NULL; - if (!(t = (retro_task_t*)calloc(1, sizeof(*t)))) + if (!(t = task_init())) { free(s); return NULL;