From b60ed132f5da4468b797f73cceaf082eecc3aad6 Mon Sep 17 00:00:00 2001 From: LibretroAdmin Date: Sun, 4 Oct 2026 10:14:56 +0000 Subject: [PATCH] gfx_widgets: task updates go to the widgets' owner, not under its lock A task's progress push took the widget state lock on the main thread and made, measured and updated the task's widget there, behind whatever frame the video thread was drawing. The widget and the task pointed at each other, so freeing either meant reaching into the other from the wrong thread at the right moment - most of the comments this removes were about that. The main thread now sends what the task looks like - title or error, progress, flags, style - and nothing else. The owner applies the updates once a frame, before it touches the queue: it finds the widget by a key, makes one if the task has none, and measures it with the font it owns. The key is a number the task carries in frontend_userdata, so nothing holds a pointer to a task or expects one to a widget; a widget freed or a task retired is simply not found. gfx_widgets_task_transfer() moves the key and tells the owner, which re-keys the widget before the old owner's finished state can start it expiring. Updates travel on a lock-free list the owner takes whole. Progress is sent every frame and gives way when the owner falls behind; a finished task's last update is always sent. Task widgets wait for room on the owner's side, and a stamp keeps their order against plain messages, which stay on the pending ring. The state lock remains for layout, fonts and lifecycle, and on a backend without pointer atomics it guards the update list. The task-gate sample now runs titled tasks to retirement against an owner thread under ThreadSanitizer; the ownership and bar-stack tests follow the updates to the owner. --- gfx/gfx_widgets.c | 806 ++++++++++-------- gfx/gfx_widgets.h | 24 +- .../gfx_widgets_task_bar_stack_test.c | 80 +- .../gfx_widgets_task_gate_test.c | 128 +-- tests-other/widget_snapshot_ownership_test.c | 228 +++-- tests-other/widget_task_lifetime_test.c | 10 +- 6 files changed, 732 insertions(+), 544 deletions(-) diff --git a/gfx/gfx_widgets.c b/gfx/gfx_widgets.c index 040e463451..d6d688f8ca 100644 --- a/gfx/gfx_widgets.c +++ b/gfx/gfx_widgets.c @@ -236,10 +236,145 @@ static disp_widget_msg_t *gfx_widgets_pending_pop(dispgfx_widget_t *p_dispwidget return msg_widget; } +static disp_widget_msg_t *gfx_widgets_pending_peek(dispgfx_widget_t *p_dispwidget) +{ + int head = retro_atomic_load_relaxed_int(&p_dispwidget->msg_queue_head); + int tail = retro_atomic_load_acquire_int(&p_dispwidget->msg_queue_tail); + if (head == tail) + return NULL; + return p_dispwidget->msg_queue[(unsigned)head & (MSG_QUEUE_PENDING_MAX - 1)]; +} + +/* A task's update on its way to the widgets' owner. The main thread + * never touches a task's widget: it sends what the task looks like + * now, and the owner finds the widget by the key the task carries in + * frontend_userdata - a number, handed on when one task passes its + * widget to another, and safe to hold after the widget or the task is + * gone. */ +typedef struct gfx_widgets_task_cmd +{ + struct gfx_widgets_task_cmd *next; + char *title; + char *error; + uintptr_t key; + uint32_t ident; + uint32_t seq; + unsigned duration; + uint8_t category; + uint8_t task_flags; + uint8_t style; + int8_t progress; + bool rebind; /* a transfer: the new owner and nothing else */ +} gfx_widgets_task_cmd_t; + +/* Progress is sent every frame and may be dropped past this many + * updates in flight; a finished task's last update never is. */ +#define TASK_CMDS_SOFT_MAX 64 + +static void gfx_widgets_task_cmd_push(dispgfx_widget_t *p_dispwidget, + gfx_widgets_task_cmd_t *cmd) +{ + retro_atomic_fetch_add_int(&p_dispwidget->task_cmds_count, 1); +#ifdef RETRO_ATOMIC_HAS_PTR + { + void *head; + do + { + head = retro_atomic_load_acquire_ptr(&p_dispwidget->task_cmds); + cmd->next = (gfx_widgets_task_cmd_t*)head; + } while (!retro_atomic_cas_ptr(&p_dispwidget->task_cmds, head, cmd)); + } +#else + gfx_widgets_state_lock(); + cmd->next = (gfx_widgets_task_cmd_t*)p_dispwidget->task_cmds; + p_dispwidget->task_cmds = cmd; + gfx_widgets_state_unlock(); +#endif +} + +/* Everything sent since the last call, oldest first. */ +static gfx_widgets_task_cmd_t *gfx_widgets_task_cmds_take( + dispgfx_widget_t *p_dispwidget) +{ + gfx_widgets_task_cmd_t *cmd, *list = NULL; +#ifdef RETRO_ATOMIC_HAS_PTR + if (!retro_atomic_load_relaxed_ptr(&p_dispwidget->task_cmds)) + return NULL; + cmd = (gfx_widgets_task_cmd_t*) + retro_atomic_exchange_ptr(&p_dispwidget->task_cmds, NULL); +#else + gfx_widgets_state_lock(); + cmd = (gfx_widgets_task_cmd_t*)p_dispwidget->task_cmds; + p_dispwidget->task_cmds = NULL; + gfx_widgets_state_unlock(); +#endif + while (cmd) + { + gfx_widgets_task_cmd_t *next = cmd->next; + cmd->next = list; + list = cmd; + cmd = next; + } + return list; +} + +static void gfx_widgets_task_cmd_free(dispgfx_widget_t *p_dispwidget, + gfx_widgets_task_cmd_t *cmd) +{ + retro_atomic_fetch_sub_int(&p_dispwidget->task_cmds_count, 1); + free(cmd->title); + free(cmd->error); + free(cmd); +} + +static void gfx_widgets_task_cmds_discard(dispgfx_widget_t *p_dispwidget) +{ + gfx_widgets_task_cmd_t *cmd = gfx_widgets_task_cmds_take(p_dispwidget); + while (cmd) + { + gfx_widgets_task_cmd_t *next = cmd->next; + gfx_widgets_task_cmd_free(p_dispwidget, cmd); + cmd = next; + } +} + +/* Task widgets not yet on screen were never animated or counted. */ +static void gfx_widgets_task_pending_discard(dispgfx_widget_t *p_dispwidget) +{ + unsigned i; + for (i = 0; i < p_dispwidget->task_pending_size; i++) + { + disp_widget_msg_t *msg_widget = p_dispwidget->task_pending[i]; + free(msg_widget->msg); + free(msg_widget->msg_new); + free(msg_widget); + } + p_dispwidget->task_pending_size = 0; +} + +/* The next message to go on screen: the older of the plain ring's + * head and the first waiting task widget. */ +static disp_widget_msg_t *gfx_widgets_pending_next(dispgfx_widget_t *p_dispwidget) +{ + disp_widget_msg_t *plain = gfx_widgets_pending_peek(p_dispwidget); + if ( p_dispwidget->task_pending_size + && (!plain || (int32_t)(p_dispwidget->task_pending[0]->seq + - plain->seq) < 0)) + { + unsigned i; + disp_widget_msg_t *msg_widget = p_dispwidget->task_pending[0]; + p_dispwidget->task_pending_size--; + for (i = 0; i < p_dispwidget->task_pending_size; i++) + p_dispwidget->task_pending[i] = p_dispwidget->task_pending[i + 1]; + return msg_widget; + } + return gfx_widgets_pending_pop(p_dispwidget); +} + /* Width, wrap and height of a plain message, from the font the widgets * draw with, on the thread that owns it: the consumer of the message * queue (gfx_widgets_iterate_frame()), not whoever pushed it. Task - * messages are measured at push, where their updates happen. */ + * messages are measured where the owner applies their updates. */ static void gfx_widgets_msg_measure(dispgfx_widget_t *p_dispwidget, disp_widget_msg_t *msg_widget) { @@ -318,10 +453,17 @@ static void gfx_widgets_msg_measure(dispgfx_widget_t *p_dispwidget, } } +/* A widget can be passed between tasks - a download hands its widget + * to the decompress task it spawns - and the per-task lifecycle flags + * are sticky, so without re-keying they would describe the previous + * owner: the widget would start dying on the strength of a task that + * is no longer the one driving it. task->ident is unique per task, so + * a mismatch is an exact test for "different owner". EXPIRED is left + * alone: a widget already on its way out stays on its way out. */ static void gfx_widgets_task_rebind(disp_widget_msg_t *msg_widget, - retro_task_t *task) + uint32_t ident) { - if (msg_widget->task_ident != task->ident) + if (msg_widget->task_ident != ident) { if (msg_widget->flags & DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED) { @@ -333,309 +475,208 @@ static void gfx_widgets_task_rebind(disp_widget_msg_t *msg_widget, msg_widget->flags &= ~(DISPWIDG_FLAG_TASK_FINISHED | DISPWIDG_FLAG_TASK_ERROR | DISPWIDG_FLAG_TASK_CANCELLED); - msg_widget->task_ident = task->ident; + msg_widget->task_ident = ident; } - msg_widget->task_ptr = task; msg_widget->flags |= DISPWIDG_FLAG_TASK; } void gfx_widgets_task_transfer(retro_task_t *from, retro_task_t *to) { - gfx_widgets_state_lock(); - to->frontend_userdata = from->frontend_userdata; + dispgfx_widget_t *p_dispwidget = &dispwidget_st; + uintptr_t key = (uintptr_t)from->frontend_userdata; + gfx_widgets_task_cmd_t *cmd; + + to->frontend_userdata = from->frontend_userdata; from->frontend_userdata = NULL; - if (to->frontend_userdata) - gfx_widgets_task_rebind((disp_widget_msg_t*)to->frontend_userdata, to); - gfx_widgets_state_unlock(); + /* The owner re-keys the widget on its next frame. Should this not + * get through, the new task's first update re-keys it instead. */ + if ( !key + || retro_atomic_load_acquire_int(&p_dispwidget->task_cmds_count) + >= TASK_CMDS_SOFT_MAX + || !(cmd = (gfx_widgets_task_cmd_t*)calloc(1, sizeof(*cmd)))) + return; + cmd->key = key; + cmd->ident = to->ident; + cmd->rebind = true; + gfx_widgets_task_cmd_push(p_dispwidget, cmd); } -static void gfx_widgets_msg_queue_push_state( - retro_task_t *task, - task_progress_snapshot_t *snapshot, - const char *msg, - size_t len, - unsigned duration, - char *title, - enum message_queue_icon icon, - enum message_queue_category category, - unsigned prio, bool flush, - bool menu_is_alive) +static disp_widget_msg_t *gfx_widgets_task_widget_find( + dispgfx_widget_t *p_dispwidget, uintptr_t key) { - disp_widget_msg_t *msg_widget = NULL; - dispgfx_widget_t *p_dispwidget = &dispwidget_st; - - /* No ring-full fast path wraps this function body: the - * update-existing branch (else clause below) does not enter the - * ring, it only mutates an already-tracked widget, so a full ring - * must not suppress it. The push site checks for room itself. - */ + size_t i; + for (i = 0; i < p_dispwidget->current_msgs_size; i++) { - /* Get current msg if it exists */ - if (task && task->frontend_userdata) + disp_widget_msg_t *msg_widget = p_dispwidget->current_msgs[i]; + if (msg_widget && msg_widget->task_key == key) + return msg_widget; + } + for (i = 0; i < p_dispwidget->task_pending_size; i++) + if (p_dispwidget->task_pending[i]->task_key == key) + return p_dispwidget->task_pending[i]; + return NULL; +} + +/* On the widgets' owner: bring the task's widget up to date, or make + * it one. Takes the update's strings. */ +static void gfx_widgets_task_cmd_apply(dispgfx_widget_t *p_dispwidget, + gfx_widgets_task_cmd_t *cmd) +{ + disp_widget_msg_t *msg_widget = + gfx_widgets_task_widget_find(p_dispwidget, cmd->key); + + if (msg_widget) + gfx_widgets_task_rebind(msg_widget, cmd->ident); + if (cmd->rebind) + return; + + if (!msg_widget) + { + char **text = (cmd->error && *cmd->error) ? &cmd->error : &cmd->title; + + /* A task with no title, or a muted one - whose only update is + * its retirement, for a widget it may have had - spawns nothing, + * and with no room to wait in the next update tries again */ + if ( !cmd->title + || (cmd->task_flags & RETRO_TASK_FLG_MUTE) + || p_dispwidget->task_pending_size + >= ARRAY_SIZE(p_dispwidget->task_pending) + || !(msg_widget = (disp_widget_msg_t*)calloc(1, sizeof(*msg_widget)))) + return; + + if (!(msg_widget->msg_new = strdup(*text))) { - msg_widget = (disp_widget_msg_t*)task->frontend_userdata; - /* msg_widgets can be passed between tasks: a download task - * hands its widget to the decompress task it spawns (see - * task_push_decompress()'s frontend_userdata argument), so the - * widget we just picked up may have been keyed to a different, - * now-retired task. - * - * The per-task lifecycle flags are sticky - they are only ever - * OR'd in, never cleared - so without re-keying here they - * describe the *previous* owner for the rest of the widget's - * life. That is not cosmetic: - * - * - gfx_widgets_iterate() arms the TASK_FINISHED_DURATION - * expiration timer for any widget flagged FINISHED or - * CANCELLED, so the widget starts dying immediately even - * though the task now driving it has barely started. - * - * - gfx_widgets_msg_queue_free() deliberately skips the - * task_ptr->frontend_userdata unlink when FINISHED is set, - * because for the task that actually finished, task_ptr is - * dangling by then. Applied to an inherited flag it skips - * the unlink for a task that is still running, leaving - * frontend_userdata pointing at the freed widget. - * - * The task then keeps pushing progress every frame and - * task_queue_push_progress() walks straight back into the freed - * block, where msg_new is whatever the allocator has since put - * there - a use-after-free read in string_is_equal() below. - * - * task->ident is unique per task (task_count++ at allocation) - * and never reused, so a mismatch is an exact test for - * "different owner". Clear the inherited state and disarm any - * expiration timer armed on the strength of it. EXPIRED is - * deliberately left alone: once the FINISHED flag is accurate, - * a widget already on its way out unlinks correctly on free. */ - gfx_widgets_task_rebind(msg_widget, task); + free(msg_widget); + return; + } + msg_widget->msg = *text; + *text = NULL; + msg_widget->msg_len = strlen(msg_widget->msg); + msg_widget->duration = cmd->duration; + msg_widget->alpha = 1.0f; + msg_widget->alternative_look = + (cmd->task_flags & RETRO_TASK_FLG_ALTERNATIVE_LOOK) != 0; + msg_widget->task_key = cmd->key; + msg_widget->task_ident = cmd->ident; + msg_widget->task_progress = cmd->progress; + msg_widget->seq = cmd->seq; + /* Default to small single line size and grow when necessary */ + msg_widget->flags = DISPWIDG_FLAG_SMALL | DISPWIDG_FLAG_TASK; + + if (cmd->category == MESSAGE_QUEUE_CATEGORY_WARNING) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_WARNING; + else if (cmd->category == MESSAGE_QUEUE_CATEGORY_ERROR) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_ERROR; + else if (cmd->category == MESSAGE_QUEUE_CATEGORY_SUCCESS) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_SUCCESS; + + if (text == &cmd->error) + msg_widget->flags |= DISPWIDG_FLAG_TASK_ERROR; + if (cmd->task_flags & RETRO_TASK_FLG_CANCELLED) + msg_widget->flags |= DISPWIDG_FLAG_TASK_CANCELLED; + if (cmd->task_flags & RETRO_TASK_FLG_FINISHED) + msg_widget->flags |= DISPWIDG_FLAG_TASK_FINISHED; + + if (cmd->style == TASK_STYLE_POSITIVE) + msg_widget->flags |= DISPWIDG_FLAG_POSITIVE; + else if (cmd->style == TASK_STYLE_NEGATIVE) + msg_widget->flags |= DISPWIDG_FLAG_NEGATIVE; + + msg_widget->width = font_driver_get_message_width( + p_dispwidget->gfx_widget_fonts.msg_queue.font, + msg_widget->msg, msg_widget->msg_len, 1.0f) + + p_dispwidget->simple_widget_padding / 2; + + /* Use big size only when needed */ + if (strchr(msg_widget->msg, '\n')) + { + msg_widget->flags &= ~DISPWIDG_FLAG_SMALL; + if (msg_widget->text_height == p_dispwidget->gfx_widget_fonts.msg_queue.line_height) + msg_widget->text_height *= 2; } - /* Spawn a new notification */ - if (!msg_widget) + p_dispwidget->task_pending[p_dispwidget->task_pending_size++] = msg_widget; + return; + } + + /* Update task info */ + if (msg_widget->flags & DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED) + { + uintptr_t _tag = (uintptr_t)&msg_widget->expiration_timer; + gfx_animation_kill_widget_by_tag(&_tag); + msg_widget->flags &= ~DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED; + } + + if (cmd->title && + !string_is_equal(cmd->title, msg_widget->msg_new + ? msg_widget->msg_new : msg_widget->msg)) + { + uintptr_t title_tag = (uintptr_t)&msg_widget->msg_transition_animation; + size_t _len; + unsigned new_width; + const char *new_title; + + gfx_animation_kill_widget_by_tag(&title_tag); + + if (msg_widget->msg_new) { - const char *msg_title = msg; - - /* A task with no title, or a muted one - whose only push is - * its retirement, for a widget it may have had - spawns - * nothing */ - if (task && (!snapshot->title - || (snapshot->flags & RETRO_TASK_FLG_MUTE))) - return; - - msg_widget = (disp_widget_msg_t*)malloc(sizeof(*msg_widget)); - - if (!msg_widget) - return; - - msg_widget->msg = NULL; - msg_widget->msg_new = NULL; - msg_widget->msg_transition_animation = 0.0f; - msg_widget->msg_len = 0; - msg_widget->duration = duration; - - msg_widget->text_height = 0; - - msg_widget->offset_y = 0; - msg_widget->alpha = 1.0f; - msg_widget->alternative_look = task && (snapshot->flags & RETRO_TASK_FLG_ALTERNATIVE_LOOK); - - msg_widget->width = 0; - - msg_widget->expiration_timer = 0; - - msg_widget->task_ptr = task; - - msg_widget->task_progress = 0; - msg_widget->task_ident = 0; - - msg_widget->hourglass_rotation = 0.0f; - msg_widget->hourglass_timer = 0.0f; - msg_widget->flags = 0; - - if (category == MESSAGE_QUEUE_CATEGORY_WARNING) - msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_WARNING; - else if (category == MESSAGE_QUEUE_CATEGORY_ERROR) - msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_ERROR; - else if (category == MESSAGE_QUEUE_CATEGORY_SUCCESS) - msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_SUCCESS; - - /* Default to small single line size and grow when necessary */ - msg_widget->flags |= DISPWIDG_FLAG_SMALL; - - if (task) - { - char **text = (snapshot->error && *snapshot->error) - ? &snapshot->error : &snapshot->title; - msg_widget->flags |= DISPWIDG_FLAG_TASK; - - if (snapshot->error && *snapshot->error) - msg_widget->flags |= DISPWIDG_FLAG_TASK_ERROR; - - msg_widget->msg_new = strdup(*text); - if (!msg_widget->msg_new) - { - free(msg_widget); - return; - } - msg_title = msg_widget->msg = *text; - *text = NULL; - len = strlen(msg_title); - msg_widget->msg_len = len; - - if ((snapshot->flags & RETRO_TASK_FLG_CANCELLED) != 0) - msg_widget->flags |= DISPWIDG_FLAG_TASK_CANCELLED; - if ((snapshot->flags & RETRO_TASK_FLG_FINISHED) != 0) - msg_widget->flags |= DISPWIDG_FLAG_TASK_FINISHED; - msg_widget->task_progress = snapshot->progress; - msg_widget->task_ident = task->ident; - - if (task->style == TASK_STYLE_POSITIVE) - msg_widget->flags |= DISPWIDG_FLAG_POSITIVE; - else if (task->style == TASK_STYLE_NEGATIVE) - msg_widget->flags |= DISPWIDG_FLAG_NEGATIVE; - - msg_widget->width = font_driver_get_message_width( - p_dispwidget->gfx_widget_fonts.msg_queue.font, - msg_title, - msg_widget->msg_len, 1.0f) + - p_dispwidget->simple_widget_padding / 2; - - task->frontend_userdata = msg_widget; - - msg_widget->hourglass_rotation = 0; - } - else - { - /* Measured by the consumer, which owns the font */ - msg_widget->msg = strdup(msg_title); - msg_widget->msg_len = len; - if (!msg_widget->msg) - { - free(msg_widget); - return; - } - } - - /* Use big size only when needed; plain messages are measured - * when the consumer takes them */ - if (task && strchr(msg_widget->msg, '\n')) - { - msg_widget->flags &= ~DISPWIDG_FLAG_SMALL; - if (msg_widget->text_height == p_dispwidget->gfx_widget_fonts.msg_queue.line_height) - msg_widget->text_height *= 2; - } - - /* One producer, so the room check and the push are one - * step by construction */ - { - bool queue_full = !gfx_widgets_pending_push(p_dispwidget, msg_widget); - - if (queue_full) - { - /* Lost the race against another producer. Roll back - * the widget we just allocated. - * - * The spawn-new branch has already published it to - * task->frontend_userdata, so that reference has to be - * dropped here regardless of the task's flags: the task - * is unambiguously alive (we are inside a call it just - * made), and leaving the pointer behind would hand the - * next progress push a freed widget. - * - * The sticky DISPWIDG_FLAG_TASK is cleared too: this - * widget never reached current_msgs, so it was never - * counted in msg_queue_tasks_count and must not - * decrement it on the way out. */ - if (task) - { - if (task->frontend_userdata == msg_widget) - task->frontend_userdata = NULL; - msg_widget->task_ptr = NULL; - msg_widget->flags &= ~DISPWIDG_FLAG_TASK; - gfx_widgets_msg_queue_free(p_dispwidget, msg_widget); - } - else - { - /* Never animated and never shown: nothing of the - * widgets' own state to unwind, and the caller need - * not hold the widget state lock */ - free(msg_widget->msg); - free(msg_widget->msg_new); - } - free(msg_widget); - return; - } - } + free(msg_widget->msg_new); + msg_widget->msg_new = NULL; + } + + new_title = msg_widget->msg_new = cmd->title; + cmd->title = NULL; + + _len = strlen(new_title); + new_width = font_driver_get_message_width( + p_dispwidget->gfx_widget_fonts.msg_queue.font, + new_title, + _len, + 1.0f); + + msg_widget->msg_len = _len; + msg_widget->msg_transition_animation = 0; + + if (!msg_widget->alternative_look) + { + gfx_animation_ctx_entry_t entry; + + entry.easing_enum = EASING_OUT_QUAD; + entry.tag = title_tag; + entry.duration = MSG_QUEUE_ANIMATION_DURATION; + entry.target_value = p_dispwidget->msg_queue_height / 2.0f; + entry.subject = &msg_widget->msg_transition_animation; + entry.cb = msg_widget_msg_transition_animation_done; + entry.userdata = msg_widget; + + gfx_animation_push_widget(&entry); } - /* Update task info */ else - { - if (msg_widget->flags & DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED) - { - uintptr_t _tag = (uintptr_t)&msg_widget->expiration_timer; - gfx_animation_kill_widget_by_tag(&_tag); - msg_widget->flags &= ~DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED; - } + msg_widget_msg_transition_animation_done(msg_widget); - if (snapshot->title && - !string_is_equal(snapshot->title, msg_widget->msg_new - ? msg_widget->msg_new : msg_widget->msg)) - { - uintptr_t title_tag = (uintptr_t)&msg_widget->msg_transition_animation; - size_t _len; - unsigned new_width; - const char *new_title; + msg_widget->width = new_width; + } - gfx_animation_kill_widget_by_tag(&title_tag); + if (cmd->error && *cmd->error) + msg_widget->flags |= DISPWIDG_FLAG_TASK_ERROR; + if (cmd->task_flags & RETRO_TASK_FLG_CANCELLED) + msg_widget->flags |= DISPWIDG_FLAG_TASK_CANCELLED; + if (cmd->task_flags & RETRO_TASK_FLG_FINISHED) + msg_widget->flags |= DISPWIDG_FLAG_TASK_FINISHED; + msg_widget->task_progress = cmd->progress; +} - if (msg_widget->msg_new) - { - free(msg_widget->msg_new); - msg_widget->msg_new = NULL; - } - - new_title = msg_widget->msg_new = snapshot->title; - snapshot->title = NULL; - - _len = strlen(new_title); - new_width = font_driver_get_message_width( - p_dispwidget->gfx_widget_fonts.msg_queue.font, - new_title, - _len, - 1.0f); - - msg_widget->msg_len = _len; - msg_widget->msg_transition_animation = 0; - - if (!msg_widget->alternative_look) - { - gfx_animation_ctx_entry_t entry; - - entry.easing_enum = EASING_OUT_QUAD; - entry.tag = title_tag; - entry.duration = MSG_QUEUE_ANIMATION_DURATION; - entry.target_value = p_dispwidget->msg_queue_height / 2.0f; - entry.subject = &msg_widget->msg_transition_animation; - entry.cb = msg_widget_msg_transition_animation_done; - entry.userdata = msg_widget; - - gfx_animation_push_widget(&entry); - } - else - msg_widget_msg_transition_animation_done(msg_widget); - - msg_widget->width = new_width; - } - - if (snapshot->error && *snapshot->error) - msg_widget->flags |= DISPWIDG_FLAG_TASK_ERROR; - if ((snapshot->flags & RETRO_TASK_FLG_CANCELLED) != 0) - msg_widget->flags |= DISPWIDG_FLAG_TASK_CANCELLED; - if ((snapshot->flags & RETRO_TASK_FLG_FINISHED) != 0) - msg_widget->flags |= DISPWIDG_FLAG_TASK_FINISHED; - msg_widget->task_progress = snapshot->progress; - } +/* On the widgets' owner, once a frame: the updates sent since the + * last one, in the order they were sent. */ +static void gfx_widgets_task_cmds_apply(dispgfx_widget_t *p_dispwidget) +{ + gfx_widgets_task_cmd_t *cmd = gfx_widgets_task_cmds_take(p_dispwidget); + while (cmd) + { + gfx_widgets_task_cmd_t *next = cmd->next; + gfx_widgets_task_cmd_apply(p_dispwidget, cmd); + gfx_widgets_task_cmd_free(p_dispwidget, cmd); + cmd = next; } } @@ -650,25 +691,92 @@ void gfx_widgets_msg_queue_push( unsigned prio, bool flush, bool menu_is_alive) { - task_progress_snapshot_t snapshot; + dispgfx_widget_t *p_dispwidget = &dispwidget_st; - /* A plain message touches only the queue, under its own lock, and - * the widget it allocates, which the consumer measures; a task's - * reads and updates the widget it may already have on screen */ + /* A plain message touches only the pending ring and the widget it + * allocates, which the consumer measures */ if (!task) { - gfx_widgets_msg_queue_push_state(task, NULL, msg, len, duration, title, - icon, category, prio, flush, menu_is_alive); + disp_widget_msg_t *msg_widget = + (disp_widget_msg_t*)calloc(1, sizeof(*msg_widget)); + + if (!msg_widget) + return; + + msg_widget->duration = duration; + msg_widget->alpha = 1.0f; + msg_widget->seq = p_dispwidget->msg_seq; + /* Default to small single line size and grow when necessary */ + msg_widget->flags = DISPWIDG_FLAG_SMALL; + + if (category == MESSAGE_QUEUE_CATEGORY_WARNING) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_WARNING; + else if (category == MESSAGE_QUEUE_CATEGORY_ERROR) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_ERROR; + else if (category == MESSAGE_QUEUE_CATEGORY_SUCCESS) + msg_widget->flags |= DISPWIDG_FLAG_CATEGORY_SUCCESS; + + /* Measured by the consumer, which owns the font */ + msg_widget->msg = strdup(msg); + msg_widget->msg_len = len; + + /* Never animated and never shown: nothing to unwind when the + * ring has no room */ + if ( !msg_widget->msg + || !gfx_widgets_pending_push(p_dispwidget, msg_widget)) + { + free(msg_widget->msg); + free(msg_widget); + return; + } + p_dispwidget->msg_seq++; return; } - /* Publish terminal flags even if copying a string fails. */ - task_get_progress_snapshot(task, &snapshot); - gfx_widgets_state_lock(); - gfx_widgets_msg_queue_push_state(task, &snapshot, msg, len, duration, - title, icon, category, prio, flush, menu_is_alive); - gfx_widgets_state_unlock(); - free(snapshot.title); - free(snapshot.error); + + /* A task's goes to the widgets' owner as it stands now */ + { + task_progress_snapshot_t snapshot; + gfx_widgets_task_cmd_t *cmd; + uintptr_t key = (uintptr_t)task->frontend_userdata; + + /* Terminal flags are published even if copying a string fails. */ + task_get_progress_snapshot(task, &snapshot); + + /* No widget to update and none to spawn; and progress, which is + * sent again next frame, gives way when the owner is behind */ + if ( (!key && (!snapshot.title + || (snapshot.flags & RETRO_TASK_FLG_MUTE))) + || (!(snapshot.flags & RETRO_TASK_FLG_FINISHED) + && retro_atomic_load_acquire_int(&p_dispwidget->task_cmds_count) + >= TASK_CMDS_SOFT_MAX) + || !(cmd = (gfx_widgets_task_cmd_t*)calloc(1, sizeof(*cmd)))) + { + free(snapshot.title); + free(snapshot.error); + return; + } + + /* A key of its own, never a task's again: a task that handed + * its widget on and pushes once more must not reach it. */ + if (!key) + { + if (!++p_dispwidget->task_key_last) + p_dispwidget->task_key_last = 1; + key = p_dispwidget->task_key_last; + task->frontend_userdata = (void*)key; + } + cmd->title = snapshot.title; + cmd->error = snapshot.error; + cmd->key = key; + cmd->ident = task->ident; + cmd->seq = p_dispwidget->msg_seq++; + cmd->duration = duration; + cmd->category = (uint8_t)category; + cmd->task_flags = snapshot.flags; + cmd->style = (uint8_t)task->style; + cmd->progress = snapshot.progress; + gfx_widgets_task_cmd_push(p_dispwidget, cmd); + } } static void gfx_widgets_move_end(void *userdata) @@ -737,33 +845,8 @@ static void gfx_widgets_msg_queue_free( uintptr_t hourglass_timer_tag = (uintptr_t)&msg->hourglass_timer; uintptr_t title_tag = (uintptr_t)&msg->msg_transition_animation; - /* Remove the reference the task has of ourself, so that its next - * progress push spawns a fresh widget instead of dereferencing the - * memory we are about to free(). - * - * Only DISPWIDG_FLAG_TASK_FINISHED marks task_ptr as potentially - * dangling: tasks are exclusively free()d by - * retro_task_internal_gather(), which always delivers a final - * progress push with RETRO_TASK_FLG_FINISHED set immediately - * beforehand. Any task we have not seen finish is therefore still - * alive and safe to write to. - * - * DISPWIDG_FLAG_TASK_ERROR and DISPWIDG_FLAG_TASK_CANCELLED carry - * no such guarantee and must not gate this. Cancellation in - * particular is purely advisory - retro_task_*_cancel() only raises - * a flag, and the handler keeps running (and keeps pushing progress, - * once per frame) until it notices. The widget, meanwhile, gets an - * expiration timer the moment the flag is observed and is gone - * TASK_FINISHED_DURATION later. Skipping the unlink for those two - * leaves task->frontend_userdata pointing into freed memory for - * the entire remaining lifetime of the task. */ - if (msg->task_ptr && !(msg->flags & DISPWIDG_FLAG_TASK_FINISHED)) - msg->task_ptr->frontend_userdata = NULL; - - msg->task_ptr = NULL; - /* Update tasks count. Keyed off the sticky flag rather than - * task_ptr, which may already have been unlinked above. */ + * task_key, which is cleared when a widget is cut loose. */ if (msg->flags & DISPWIDG_FLAG_TASK) { if (p_dispwidget->msg_queue_tasks_count > 0) @@ -1304,6 +1387,8 @@ static INLINE void gfx_widgets_iterate_frame( /* Messages queue */ + gfx_widgets_task_cmds_apply(p_dispwidget); + /* Consume one message if available. current_msgs[] and the MOVING * flag belong to this thread, the one that owns the widgets. The * pending ring is shared with its producer and popped without a @@ -1312,7 +1397,7 @@ static INLINE void gfx_widgets_iterate_frame( if ( !(p_dispwidget->flags & DISPGFX_WIDGET_FLAG_MOVING) && (p_dispwidget->current_msgs_size < ARRAY_SIZE(p_dispwidget->current_msgs))) { - disp_widget_msg_t *msg_widget = gfx_widgets_pending_pop(p_dispwidget); + disp_widget_msg_t *msg_widget = gfx_widgets_pending_next(p_dispwidget); if (msg_widget) { @@ -2314,25 +2399,11 @@ static void gfx_widgets_free(dispgfx_widget_t *p_dispwidget) if (!msg_widget) break; - /* Note: task_ptr is deliberately left intact here. - * gfx_widgets_free() is NOT only reached from main_exit(): - * driver_uninit(), retroarch_deinit_drivers() and a user - * toggling notification widgets off all call - * gfx_widgets_deinit(false) with tasks still in flight. Blanket - * unsetting task_ptr suppressed the unlink in - * gfx_widgets_msg_queue_free(), so every live task kept a - * frontend_userdata pointer to the widget freed just below. - * gfx_widgets_msg_queue_free() applies the TASK_FINISHED rule - * instead, which is safe in both situations. - * - * The sticky DISPWIDG_FLAG_TASK is cleared first: these widgets - * never reached current_msgs, so they were never counted in - * msg_queue_tasks_count and must not decrement it. */ - msg_widget->flags &= ~DISPWIDG_FLAG_TASK; - gfx_widgets_msg_queue_free(p_dispwidget, msg_widget); free(msg_widget); } + gfx_widgets_task_cmds_discard(p_dispwidget); + gfx_widgets_task_pending_discard(p_dispwidget); /* Purge everything from the list */ @@ -2343,9 +2414,6 @@ static void gfx_widgets_free(dispgfx_widget_t *p_dispwidget) if (!msg) continue; - /* See the note in the fifo purge above: task_ptr is left for - * gfx_widgets_msg_queue_free() to unlink under the - * TASK_FINISHED rule. */ gfx_widgets_msg_queue_free(p_dispwidget, msg); free(msg); p_dispwidget->current_msgs[i] = NULL; @@ -2555,6 +2623,8 @@ bool gfx_widgets_init( retro_atomic_int_init(&p_dispwidget->msg_queue_head, 0); retro_atomic_int_init(&p_dispwidget->msg_queue_tail, 0); + gfx_widgets_task_cmds_discard(p_dispwidget); + p_dispwidget->task_pending_size = 0; memset(&p_dispwidget->current_msgs[0], 0, sizeof(p_dispwidget->current_msgs)); p_dispwidget->current_msgs_size = 0; @@ -2670,61 +2740,47 @@ static void gfx_widgets_context_destroy(dispgfx_widget_t *p_dispwidget) gfx_widgets_font_free(&p_dispwidget->gfx_widget_fonts.msg_queue); } -/* Severs the two-way link between every notification widget and its - * task. +/* Cuts every notification widget loose from its task. * * Must run before p_dispwidget->active goes false and progress pushes * stop reaching us. Display widgets persist across driver reinits by - * default (dispgfx_widget_t.persisting), so a task that finishes - * while we are inactive gets retired and free()d with the widget still - * holding a task_ptr to it, and DISPWIDG_FLAG_TASK_FINISHED never set - * to warn us off. This is the last point at which the link can be - * dropped safely: any task we have not seen finish is still alive, - * because pushes are only about to stop, not already stopped. + * default (dispgfx_widget_t.persisting), and a task that finishes + * while we are inactive is never seen to: its widget would wait for an + * update that cannot come. * - * Widgets severed from a still-running task are marked expired. They - * can no longer be updated, and a running task never reaches the state - * that would start their expiration timer, so they would otherwise - * linger indefinitely next to the fresh widget the task spawns on its - * first push after reinit. */ + * Widgets cut loose from a task not seen to finish are marked expired, + * so they do not linger next to the fresh widget the task spawns on + * its first push after reinit. */ static void gfx_widgets_detach_tasks(dispgfx_widget_t *p_dispwidget) { size_t i; - /* Widgets still in the fifo have not been displayed yet and cannot - * be reached individually, so discard them outright. At most one - * frame's worth can be queued: gfx_widgets_iterate() drains the - * fifo every frame. */ + /* Not displayed yet: discarded outright, with the updates nobody + * will apply. */ for (;;) { - disp_widget_msg_t *msg_widget = NULL; - - msg_widget = gfx_widgets_pending_pop(p_dispwidget); + disp_widget_msg_t *msg_widget = gfx_widgets_pending_pop(p_dispwidget); if (!msg_widget) break; - /* Never entered current_msgs, so never counted in - * msg_queue_tasks_count */ - msg_widget->flags &= ~DISPWIDG_FLAG_TASK; gfx_widgets_msg_queue_free(p_dispwidget, msg_widget); free(msg_widget); } + gfx_widgets_task_cmds_discard(p_dispwidget); + gfx_widgets_task_pending_discard(p_dispwidget); for (i = 0; i < p_dispwidget->current_msgs_size; i++) { disp_widget_msg_t *msg = p_dispwidget->current_msgs[i]; - if (!msg || !msg->task_ptr) + if (!msg || !msg->task_key) continue; if (!(msg->flags & DISPWIDG_FLAG_TASK_FINISHED)) - { - msg->task_ptr->frontend_userdata = NULL; - msg->flags |= DISPWIDG_FLAG_EXPIRED; - } + msg->flags |= DISPWIDG_FLAG_EXPIRED; - msg->task_ptr = NULL; + msg->task_key = 0; } } diff --git a/gfx/gfx_widgets.h b/gfx/gfx_widgets.h index 774e714e75..0279d1a61d 100644 --- a/gfx/gfx_widgets.h +++ b/gfx/gfx_widgets.h @@ -128,8 +128,8 @@ enum disp_widget_flags_enum /* Size */ DISPWIDG_FLAG_SMALL = (1 << 11), /* Was this widget spawned by a task? Sticky for the lifetime of - * the widget; unlike task_ptr, which is a liveness link that may - * legitimately be cleared while the widget is still on screen. */ + * the widget; unlike task_key, which is cleared when the widget is + * cut loose from its task while still on screen. */ DISPWIDG_FLAG_TASK = (1 << 12) }; @@ -174,9 +174,12 @@ typedef struct disp_widget_msg { char *msg; char *msg_new; - retro_task_t *task_ptr; + /* What a task's updates find this widget by: the value the task + * carries in frontend_userdata. Never a pointer to the task. */ + uintptr_t task_key; uint32_t task_ident; + uint32_t seq; /* arrival order, across plain and task messages */ size_t msg_len; unsigned duration; unsigned text_height; @@ -238,6 +241,21 @@ typedef struct dispgfx_widget disp_widget_msg_t* msg_queue[MSG_QUEUE_PENDING_MAX]; retro_atomic_int_t msg_queue_head; retro_atomic_int_t msg_queue_tail; + /* Task updates on their way to the widgets' owner: the main thread + * pushes, the owner takes the lot once a frame. */ +#ifdef RETRO_ATOMIC_HAS_PTR + retro_atomic_ptr_t task_cmds; +#else + void *task_cmds; +#endif + retro_atomic_int_t task_cmds_count; + /* The owner's: task widgets waiting for room on screen. */ + disp_widget_msg_t* task_pending[MSG_QUEUE_PENDING_MAX]; + unsigned task_pending_size; + /* The main thread's: stamps messages in the order they are pushed, + * and the last key handed to a task. */ + uint32_t msg_seq; + uintptr_t task_key_last; disp_widget_msg_t* current_msgs[MSG_QUEUE_ONSCREEN_MAX]; gfx_widget_fonts_t gfx_widget_fonts; /* ptr alignment */ diff --git a/samples/gfx/gfx_widgets_task_bar_stack/gfx_widgets_task_bar_stack_test.c b/samples/gfx/gfx_widgets_task_bar_stack/gfx_widgets_task_bar_stack_test.c index ad7ecd4467..71054c420e 100644 --- a/samples/gfx/gfx_widgets_task_bar_stack/gfx_widgets_task_bar_stack_test.c +++ b/samples/gfx/gfx_widgets_task_bar_stack/gfx_widgets_task_bar_stack_test.c @@ -81,6 +81,24 @@ static void pump(int frames) } } +/* The widget a task's updates reach, on screen or waiting for room: + * the owner finds it by the key the task carries. */ +static disp_widget_msg_t *task_widget(const retro_task_t *task) +{ + size_t i; + dispgfx_widget_t *p = dispwidget_get_ptr(); + uintptr_t key = (uintptr_t)task->frontend_userdata; + if (!key) + return NULL; + for (i = 0; i < p->current_msgs_size; i++) + if (p->current_msgs[i] && p->current_msgs[i]->task_key == key) + return p->current_msgs[i]; + for (i = 0; i < p->task_pending_size; i++) + if (p->task_pending[i]->task_key == key) + return p->task_pending[i]; + return NULL; +} + static void task_push(retro_task_t *task) { gfx_widgets_msg_queue_push(task, NULL, 0, 0, NULL, @@ -224,8 +242,8 @@ static void test_title_churn(bool alternative) gfx_widgets_deinit(false); } -/* Tear down with task widgets still outstanding, so msg_queue_free - * runs against a live task->frontend_userdata. */ +/* Tear down with task widgets still outstanding: no widget may go on + * answering to a task afterwards. */ static void test_deinit_with_live_tasks(void) { int c, i; @@ -248,10 +266,10 @@ static void test_deinit_with_live_tasks(void) pump(3); gfx_widgets_deinit((c & 1) ? true : false); - /* The widgets are gone; the tasks must not still point at them. */ + /* Gone, or cut loose: the tasks' keys must find nothing. */ for (i = 0; i < 4; i++) - CHECK(tasks[i].frontend_userdata == NULL, - "task kept a pointer into a freed widget after deinit"); + CHECK(task_widget(&tasks[i]) == NULL, + "a widget still answered to its task after deinit"); } } @@ -292,17 +310,17 @@ static void test_suppressed_completion(void) task->handler = finish_silent_task; task_push(task); pump(2); - widget = (disp_widget_msg_t*)task->frontend_userdata; + widget = task_widget(task); CHECK(widget != NULL, "completion task has no widget"); if (muted) task_set_flags(task, RETRO_TASK_FLG_MUTE, true); task_queue_push(task); task_queue_check(); - /* task has been retired; only the widget remains valid. */ + /* task has been retired; its last update is the owner's to apply */ + pump(2); if (widget) CHECK(widget->flags & DISPWIDG_FLAG_TASK_FINISHED, - "widget retained a live link to a retired task"); - pump(2); + "widget never learned its task had finished"); gfx_widgets_deinit(false); } } @@ -325,16 +343,17 @@ static void test_delayed_handoff(void) download.flags |= RETRO_TASK_FLG_FINISHED | RETRO_TASK_FLG_CANCELLED; download.error = "old download error"; task_push(&download); - widget = (disp_widget_msg_t*)download.frontend_userdata; - CHECK(widget != NULL, "download has no handoff widget"); - if (!widget) - { - gfx_widgets_deinit(false); - continue; - } + widget = NULL; if (displayed) { pump(2); + widget = task_widget(&download); + CHECK(widget != NULL, "download has no handoff widget"); + if (!widget) + { + gfx_widgets_deinit(false); + continue; + } CHECK(widget->flags & DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED, "finished download did not arm expiration"); } @@ -346,17 +365,36 @@ static void test_delayed_handoff(void) #else gfx_widgets_task_transfer(&download, &extraction); #endif + /* The owner's next frame takes the handoff before anything + * else: with nothing displayed yet, the download's update and + * the handoff arrive together. */ + pump(1); if (displayed) CHECK(stub_last_killed_animation_tag == (uintptr_t)&widget->expiration_timer, "handoff did not cancel the old expiration callback"); CHECK(!download.frontend_userdata, "download kept ownership"); - CHECK(widget->task_ptr == &extraction && widget->task_ident == 101, + CHECK(!displayed || task_widget(&extraction) == widget, + "handoff moved the task to another widget"); + widget = task_widget(&extraction); + CHECK(widget != NULL, "extraction has no handoff widget"); + if (!widget) + { + gfx_widgets_deinit(false); + continue; + } + CHECK(widget->task_ident == 101, "handoff waits for extraction progress to bind its owner"); CHECK(!(widget->flags & (DISPWIDG_FLAG_TASK_FINISHED | DISPWIDG_FLAG_TASK_CANCELLED | DISPWIDG_FLAG_TASK_ERROR | DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED)), "handoff retained download lifetime state"); + /* The download pushing once more gets a widget of its own */ + task_push(&download); + pump(1); + CHECK(task_widget(&download) != widget + && !(widget->flags & DISPWIDG_FLAG_TASK_FINISHED), + "the old owner's push reached the widget it handed on"); pump(180); CHECK(!(widget->flags & DISPWIDG_FLAG_EXPIRATION_TIMER_STARTED), "delayed extraction armed inherited expiration"); @@ -364,12 +402,14 @@ static void test_delayed_handoff(void) { extraction.progress = 37; task_push(&extraction); - CHECK(widget->task_ptr == &extraction && widget->task_progress == 37, + pump(1); + CHECK(task_widget(&extraction) == widget + && widget->task_progress == 37, "extraction progress did not update the inherited widget"); } gfx_widgets_deinit(false); - CHECK(!extraction.frontend_userdata, - "teardown left extraction pointing at a freed widget"); + CHECK(task_widget(&extraction) == NULL, + "teardown left a widget answering to extraction"); } } diff --git a/samples/gfx/gfx_widgets_task_gate/gfx_widgets_task_gate_test.c b/samples/gfx/gfx_widgets_task_gate/gfx_widgets_task_gate_test.c index d9a8c6a626..48c6eb892c 100644 --- a/samples/gfx/gfx_widgets_task_gate/gfx_widgets_task_gate_test.c +++ b/samples/gfx/gfx_widgets_task_gate/gfx_widgets_task_gate_test.c @@ -1,18 +1,17 @@ -/* The task queue's retirement push against the widgets' owner. +/* The task queue's pushes against the widgets' owner. * - * Under the threaded video wrapper the widgets are freed on the video - * thread, and a widget bound to a task that has not finished unlinks - * itself there: it clears task->frontend_userdata, under the widgets' - * state lock. The task queue's gather, on the main thread, decides - * whether a finished task with nothing to say still needs a push to - * retire its widget - and must not decide that by reading the same - * field without that lock. + * Under the threaded video wrapper the widgets are made, updated and + * freed on the video thread. The main thread's gather never touches a + * widget: each push sends what the task looks like to the owner, which + * finds the widget by the key the task carries. The gather retires and + * frees a task while the owner may still be applying its updates, so + * an update holds nothing of the task's. * - * Each round binds a widget to a muted task that finishes, then frees - * the widget on a second thread, as the video thread does, while the - * main thread's gather retires the task through the widgets' own push. - * Built under ThreadSanitizer: an unguarded read of the link is a - * reported race; under ASan, a widget writing into a task already + * Each round runs a titled task from its first push to its retirement + * on the main thread while a second thread applies the updates and + * frees the widgets, as the video thread does. Built under + * ThreadSanitizer: anything the two sides share without ordering is a + * reported race; under ASan, an update reaching into a task already * retired is a use after free. */ #include @@ -26,12 +25,24 @@ #define ROUNDS 300 static video_driver_state_t fake_video_st; -static retro_atomic_int_t go; +static retro_atomic_int_t stop; static unsigned pushes; +static unsigned spawned; +static unsigned passes; +static bool retired; -static void finish_handler(retro_task_t *task) +static void progress_handler(retro_task_t *task) { - task_set_flags(task, RETRO_TASK_FLG_FINISHED, true); + task_set_progress(task, (int8_t)(passes * 30)); + if (++passes == 3) + task_set_flags(task, RETRO_TASK_FLG_FINISHED, true); +} + +static void retired_cb(retro_task_t *task, void *task_data, + void *user_data, const char *err) +{ + (void)task; (void)task_data; (void)user_data; (void)err; + retired = true; } /* What runloop_task_msg_queue_push() does with widgets active */ @@ -44,70 +55,91 @@ static void msg_push(retro_task_t *task, const char *msg, prio, flush, false); } -static void owner_frees(void *data) +/* The owner's frame: apply what was sent; the widgets made go + * straight back, as if shown and expired. */ +static void owner_frame(void) { - disp_widget_msg_t *w = (disp_widget_msg_t*)data; - while (!retro_atomic_load_acquire_int(&go)) { } gfx_widgets_state_lock(); - gfx_widgets_msg_queue_free(&dispwidget_st, w); + gfx_widgets_task_cmds_apply(&dispwidget_st); + spawned += dispwidget_st.task_pending_size; + gfx_widgets_task_pending_discard(&dispwidget_st); gfx_widgets_state_unlock(); } +static void owner_runs(void *unused) +{ + (void)unused; + while (!retro_atomic_load_acquire_int(&stop)) + owner_frame(); +} + int main(void) { unsigned i; + sthread_t *owner; - dispwidget_st.state_lock = slock_new(); + dispwidget_st.state_lock = slock_new(); fake_video_st.thread_wrapper_active = true; - dispwidget_st.video_st = &fake_video_st; + dispwidget_st.video_st = &fake_video_st; task_queue_init(false, msg_push); + owner = sthread_create(owner_runs, NULL); + if (!owner) + return 1; + for (i = 0; i < ROUNDS; i++) { - sthread_t *t; - retro_task_t *task = task_init(); - disp_widget_msg_t *w = (disp_widget_msg_t*)calloc(1, sizeof(*w)); - if (!task || !w) + retro_task_t *task = task_init(); + if (!task) return 1; - task->handler = finish_handler; - task->flags |= RETRO_TASK_FLG_MUTE; - w->flags = DISPWIDG_FLAG_TASK; - w->task_ptr = task; - task->frontend_userdata = w; - - retro_atomic_store_release_int(&go, 0); - t = sthread_create(owner_frees, w); + task->handler = progress_handler; + task->callback = retired_cb; + task->title = strdup("a task"); + passes = 0; + retired = false; task_queue_push(task); - retro_atomic_store_release_int(&go, 1); - task_queue_check(); - sthread_join(t); - free(w); + while (!retired) + task_queue_check(); + } + + retro_atomic_store_release_int(&stop, 1); + sthread_join(owner); + owner_frame(); + + if ( !spawned + || retro_atomic_load_acquire_int(&dispwidget_st.task_cmds_count)) + { + printf("FAIL %u widgets made, %d updates left unapplied\n", spawned, + (int)retro_atomic_load_acquire_int(&dispwidget_st.task_cmds_count)); + return 1; } /* A muted task with a title and nothing on screen: its retirement - * push carries no text and must not put a widget up. */ + * push carries no text and must not send for a widget. */ { retro_task_t *task = task_init(); - int before = retro_atomic_load_acquire_int( - &dispwidget_st.msg_queue_tail); if (!task) return 1; - task->handler = finish_handler; + task->handler = progress_handler; + task->callback = retired_cb; task->title = strdup("muted"); task->flags |= RETRO_TASK_FLG_MUTE; + passes = 2; + retired = false; task_queue_push(task); - task_queue_check(); - if (retro_atomic_load_acquire_int(&dispwidget_st.msg_queue_tail) - != before) + while (!retired) + task_queue_check(); + if (retro_atomic_load_acquire_int(&dispwidget_st.task_cmds_count)) { - printf("FAIL a muted task's retirement put up a widget\n"); + printf("FAIL a muted task's retirement sent for a widget\n"); return 1; } } task_queue_deinit(); slock_free(dispwidget_st.state_lock); - printf("ok %u tasks retired while their widgets were freed on another thread (%u pushes)\n", - ROUNDS, pushes); + printf("ok %u tasks pushed and retired while their widgets were " + "made and freed on another thread (%u pushes, %u widgets)\n", + ROUNDS, pushes, spawned); return 0; } diff --git a/tests-other/widget_snapshot_ownership_test.c b/tests-other/widget_snapshot_ownership_test.c index ace238d288..726b6fa2cc 100644 --- a/tests-other/widget_snapshot_ownership_test.c +++ b/tests-other/widget_snapshot_ownership_test.c @@ -57,11 +57,21 @@ static char *checked_strdup(const char *text) return copy; } +static void *checked_calloc(size_t count, size_t size) +{ + void *ptr = checked_malloc(count * size); + if (ptr) + memset(ptr, 0, count * size); + return ptr; +} + #define malloc checked_malloc +#define calloc checked_calloc #define free checked_free #define strdup checked_strdup #include "../gfx/gfx_widgets.c" #undef malloc +#undef calloc #undef free #undef strdup @@ -98,74 +108,85 @@ bool gfx_animation_push_widget(gfx_animation_ctx_entry_t *entry) return true; } -static void push(retro_task_t *task, task_progress_snapshot_t *snapshot) +static void push(gfx_widgets_task_cmd_t *cmd) { - gfx_widgets_msg_queue_push_state(task, snapshot, "message", 7, 60, - NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO, - 1, false, false); + gfx_widgets_task_cmd_apply(&dispwidget_st, cmd); } -static void release_snapshot(task_progress_snapshot_t *snapshot) +static void release_cmd(gfx_widgets_task_cmd_t *cmd) { - checked_free(snapshot->title); - checked_free(snapshot->error); - snapshot->title = NULL; - snapshot->error = NULL; + checked_free(cmd->title); + checked_free(cmd->error); + cmd->title = NULL; + cmd->error = NULL; +} + +/* The waiting task widget goes on screen, where updates find it. */ +static disp_widget_msg_t *take_widget(void) +{ + disp_widget_msg_t *widget; + if (!dispwidget_st.task_pending_size) + return NULL; + widget = dispwidget_st.task_pending[0]; + dispwidget_st.task_pending_size = 0; + dispwidget_st.current_msgs[0] = widget; + dispwidget_st.current_msgs_size = 1; + return widget; } static void release_widget(disp_widget_msg_t *widget) { gfx_widgets_msg_queue_free(&dispwidget_st, widget); assert(pending_count == 0); + dispwidget_st.current_msgs[0] = NULL; + dispwidget_st.current_msgs_size = 0; checked_free(widget); } int main(void) { - retro_task_t task; - task_progress_snapshot_t snapshot; + gfx_widgets_task_cmd_t cmd; disp_widget_msg_t *widget; char *text; unsigned before; unsigned i; - memset(&task, 0, sizeof(task)); - memset(&snapshot, 0, sizeof(snapshot)); - task.ident = 1; - retro_atomic_int_init(&dispwidget_st.msg_queue_head, 0); - retro_atomic_int_init(&dispwidget_st.msg_queue_tail, 0); + memset(&cmd, 0, sizeof(cmd)); + cmd.key = 2; + cmd.ident = 1; + cmd.duration = 60; + cmd.category = MESSAGE_QUEUE_CATEGORY_INFO; - /* Neither widget allocation failure nor duplicate failure publishes - * a task association or consumes the caller's snapshot. */ + /* Neither widget allocation failure nor duplicate failure makes a + * widget or consumes the update's text. */ for (i = 1; i <= 2; i++) { - snapshot.title = checked_strdup("initial"); - text = snapshot.title; + cmd.title = checked_strdup("initial"); + text = cmd.title; fail_after = i; - push(&task, &snapshot); - assert(!task.frontend_userdata); - assert(snapshot.title == text); - assert(!gfx_widgets_pending_pop(&dispwidget_st)); - release_snapshot(&snapshot); + push(&cmd); + assert(cmd.title == text); + assert(!take_widget()); + release_cmd(&cmd); assert(live_allocations == 0); } - snapshot.title = checked_strdup("initial"); - text = snapshot.title; + cmd.title = checked_strdup("initial"); + text = cmd.title; before = duplicate_calls; - push(&task, &snapshot); + push(&cmd); assert(duplicate_calls == before + 1); - assert(!snapshot.title); - widget = gfx_widgets_pending_pop(&dispwidget_st); - assert(widget && widget == task.frontend_userdata); + assert(!cmd.title); + widget = take_widget(); + assert(widget && widget->task_key == cmd.key); assert(widget->msg == text); assert(strcmp(widget->msg_new, "initial") == 0); - snapshot.title = checked_strdup("initial"); - text = snapshot.title; + cmd.title = checked_strdup("initial"); + text = cmd.title; before = duplicate_calls; - push(&task, &snapshot); - assert(snapshot.title == text && duplicate_calls == before); - release_snapshot(&snapshot); + push(&cmd); + assert(cmd.title == text && duplicate_calls == before); + release_cmd(&cmd); /* A movement tween must survive replacing the title tween. */ memset(&pending[0], 0, sizeof(pending[0])); @@ -175,12 +196,12 @@ int main(void) /* Changed titles and animation completion need no new allocation. */ for (i = 0; i < 2; i++) { - snapshot.title = checked_strdup(i ? "latest" : "replacement"); - text = snapshot.title; + cmd.title = checked_strdup(i ? "latest" : "replacement"); + text = cmd.title; before = duplicate_calls; fail_after = 1; - push(&task, &snapshot); - assert(!snapshot.title && widget->msg_new == text); + push(&cmd); + assert(!cmd.title && widget->msg_new == text); assert(pending_count == 2); assert(pending[0].tag == (uintptr_t)widget); assert(duplicate_calls == before && fail_after == 1); @@ -199,85 +220,106 @@ int main(void) fail_after = 0; /* Completed transitions compare against the displayed title. */ - snapshot.title = checked_strdup("latest"); + cmd.title = checked_strdup("latest"); before = animation_calls; - push(&task, &snapshot); - assert(snapshot.title && !widget->msg_new); + push(&cmd); + assert(cmd.title && !widget->msg_new); assert(widget->msg == text && animation_calls == before); - release_snapshot(&snapshot); + release_cmd(&cmd); /* A missing title must still publish the final task state. */ - snapshot.flags = RETRO_TASK_FLG_FINISHED; - snapshot.progress = 100; - push(&task, &snapshot); + cmd.task_flags = RETRO_TASK_FLG_FINISHED; + cmd.progress = 100; + push(&cmd); assert(widget->flags & DISPWIDG_FLAG_TASK_FINISHED); assert(widget->task_progress == 100); assert(strcmp(widget->msg, "latest") == 0); + + /* Another task taking the widget over starts it afresh. */ + cmd.task_flags = 0; + cmd.ident = 7; + cmd.rebind = true; + push(&cmd); + assert(!(widget->flags & DISPWIDG_FLAG_TASK_FINISHED)); + assert(widget->task_ident == 7 && widget->task_progress == 100); + cmd.rebind = false; + cmd.ident = 1; release_widget(widget); - task.frontend_userdata = NULL; assert(live_allocations == 0); /* Error text is transferred instead of the unused title. */ - snapshot.flags = 0; - snapshot.title = checked_strdup("title"); - snapshot.error = checked_strdup("error"); - text = snapshot.error; - push(&task, &snapshot); - widget = gfx_widgets_pending_pop(&dispwidget_st); + cmd.title = checked_strdup("title"); + cmd.error = checked_strdup("error"); + text = cmd.error; + push(&cmd); + widget = take_widget(); assert(widget && widget->msg == text); - assert(!snapshot.error && snapshot.title); + assert(!cmd.error && cmd.title); assert(widget->flags & DISPWIDG_FLAG_TASK_ERROR); - release_snapshot(&snapshot); + release_cmd(&cmd); release_widget(widget); - assert(!task.frontend_userdata && live_allocations == 0); + assert(live_allocations == 0); /* Alternative-look widgets complete the transition synchronously. */ - snapshot.flags = RETRO_TASK_FLG_ALTERNATIVE_LOOK; - snapshot.title = checked_strdup("alternative"); - push(&task, &snapshot); - widget = gfx_widgets_pending_pop(&dispwidget_st); + cmd.task_flags = RETRO_TASK_FLG_ALTERNATIVE_LOOK; + cmd.title = checked_strdup("alternative"); + push(&cmd); + widget = take_widget(); assert(widget && widget->alternative_look); - snapshot.title = checked_strdup("immediate"); - text = snapshot.title; + cmd.title = checked_strdup("immediate"); + text = cmd.title; before = duplicate_calls; - push(&task, &snapshot); - assert(!snapshot.title && !widget->msg_new && widget->msg == text); + push(&cmd); + assert(!cmd.title && !widget->msg_new && widget->msg == text); assert(duplicate_calls == before); - snapshot.title = checked_strdup("immediate"); - push(&task, &snapshot); - assert(snapshot.title && !widget->msg_new && widget->msg == text); - release_snapshot(&snapshot); - snapshot.title = checked_strdup("changed again"); - text = snapshot.title; - push(&task, &snapshot); - assert(!snapshot.title && !widget->msg_new && widget->msg == text); + cmd.title = checked_strdup("immediate"); + push(&cmd); + assert(cmd.title && !widget->msg_new && widget->msg == text); + release_cmd(&cmd); + cmd.title = checked_strdup("changed again"); + text = cmd.title; + push(&cmd); + assert(!cmd.title && !widget->msg_new && widget->msg == text); release_widget(widget); - assert(!task.frontend_userdata && live_allocations == 0); + assert(live_allocations == 0); /* Teardown also cancels an unfinished title transition. */ - snapshot.flags = 0; - snapshot.title = checked_strdup("before teardown"); - push(&task, &snapshot); - widget = gfx_widgets_pending_pop(&dispwidget_st); + cmd.task_flags = 0; + cmd.title = checked_strdup("before teardown"); + push(&cmd); + widget = take_widget(); assert(widget); - snapshot.title = checked_strdup("pending teardown"); - push(&task, &snapshot); + cmd.title = checked_strdup("pending teardown"); + push(&cmd); assert(pending_count == 1); release_widget(widget); - assert(!task.frontend_userdata && live_allocations == 0); - - /* A full pending ring rolls back the transferred text and task link: - * full is tail - head == MSG_QUEUE_PENDING_MAX. */ - retro_atomic_store_release_int(&dispwidget_st.msg_queue_tail, - retro_atomic_load_acquire_int(&dispwidget_st.msg_queue_head) - + MSG_QUEUE_PENDING_MAX); - snapshot.title = checked_strdup("full"); - push(&task, &snapshot); - assert(!snapshot.title && !task.frontend_userdata); - release_snapshot(&snapshot); assert(live_allocations == 0); - retro_atomic_store_release_int(&dispwidget_st.msg_queue_tail, - retro_atomic_load_acquire_int(&dispwidget_st.msg_queue_head)); + + /* A muted task, or one with no title, makes no widget. */ + cmd.task_flags = RETRO_TASK_FLG_MUTE; + cmd.title = checked_strdup("muted"); + push(&cmd); + assert(cmd.title && !take_widget()); + release_cmd(&cmd); + cmd.task_flags = RETRO_TASK_FLG_FINISHED; + push(&cmd); + assert(!take_widget() && live_allocations == 0); + + /* With no room to wait in, nothing is made and the text is left. */ + cmd.task_flags = 0; + { + static disp_widget_msg_t filler; + for (i = 0; i < MSG_QUEUE_PENDING_MAX; i++) + dispwidget_st.task_pending[i] = &filler; + } + dispwidget_st.task_pending_size = MSG_QUEUE_PENDING_MAX; + cmd.title = checked_strdup("full"); + text = cmd.title; + push(&cmd); + assert(cmd.title == text); + release_cmd(&cmd); + assert(live_allocations == 0); + dispwidget_st.task_pending_size = 0; puts("widget snapshot ownership tests passed"); return 0; } diff --git a/tests-other/widget_task_lifetime_test.c b/tests-other/widget_task_lifetime_test.c index 8140a5ba89..df7a2f4074 100644 --- a/tests-other/widget_task_lifetime_test.c +++ b/tests-other/widget_task_lifetime_test.c @@ -31,10 +31,10 @@ static void draw_and_free(int8_t progress, bool alternative, widget.task_progress = progress; widget.alternative_look = alternative; widget.flags = flags | DISPWIDG_FLAG_TASK | DISPWIDG_FLAG_SMALL; - widget.task_ptr = &task; + widget.task_key = 1; - /* Model memory no longer belonging to the retired task. Cleanup - * must not write through task_ptr after the finished publication. */ + /* A widget holds a key, never the task: cleanup leaves the task + * alone, retired or not. */ task.frontend_userdata = &task; dispwidget_st.msg_queue_height = 32; dispwidget_st.gfx_widget_fonts.msg_queue.glyph_width = 10; @@ -45,8 +45,8 @@ static void draw_and_free(int8_t progress, bool alternative, "renderer changed task lifetime state"); CHECK(widget.task_progress == progress, "renderer changed progress"); gfx_widgets_msg_queue_free(&dispwidget_st, &widget); - CHECK(task.frontend_userdata == (finished ? (void*)&task : NULL), - "cleanup accessed a retired task or failed to unlink a live task"); + CHECK(task.frontend_userdata == (void*)&task, + "cleanup wrote to the task"); } int main(void)