Merge pull request #10861 from Jamiras/gfx_threaded_msg

(widgets) prevent freeing message while it's being rendered
This commit is contained in:
Autechre 2020-06-16 22:18:48 +02:00 committed by GitHub
commit 5f8419b8d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 131 additions and 113 deletions

View File

@ -18,7 +18,6 @@
#include <retro_miscellaneous.h> #include <retro_miscellaneous.h>
#include <retro_inline.h> #include <retro_inline.h>
#include <lists/file_list.h>
#include <queues/fifo_queue.h> #include <queues/fifo_queue.h>
#include <file/file_path.h> #include <file/file_path.h>
#include <streams/file_stream.h> #include <streams/file_stream.h>
@ -200,7 +199,7 @@ unsigned gfx_widgets_get_last_video_height(void *data)
size_t gfx_widgets_get_msg_queue_size(void *data) size_t gfx_widgets_get_msg_queue_size(void *data)
{ {
dispgfx_widget_t *p_dispwidget = (dispgfx_widget_t*)data; dispgfx_widget_t *p_dispwidget = (dispgfx_widget_t*)data;
return p_dispwidget->current_msgs ? p_dispwidget->current_msgs->size : 0; return p_dispwidget->current_msgs_size;
} }
/* Widgets list */ /* Widgets list */
@ -465,10 +464,11 @@ static void gfx_widgets_msg_queue_move(dispgfx_widget_t *p_dispwidget)
/* there should always be one and only one unfolded message */ /* there should always be one and only one unfolded message */
menu_widget_msg_t *unfold = NULL; menu_widget_msg_t *unfold = NULL;
for (i = (int)(p_dispwidget->current_msgs->size-1); i >= 0; i--) SLOCK_LOCK(p_dispwidget->current_msgs_lock);
for (i = (int)(p_dispwidget->current_msgs_size - 1); i >= 0; i--)
{ {
menu_widget_msg_t *msg = (menu_widget_msg_t*) menu_widget_msg_t* msg = p_dispwidget->current_msgs[i];
p_dispwidget->current_msgs->list[i].userdata;
if (!msg || msg->dying) if (!msg || msg->dying)
continue; continue;
@ -496,13 +496,14 @@ static void gfx_widgets_msg_queue_move(dispgfx_widget_t *p_dispwidget)
p_dispwidget->widgets_moving = true; p_dispwidget->widgets_moving = true;
} }
} }
SLOCK_UNLOCK(p_dispwidget->current_msgs_lock);
} }
static void gfx_widgets_msg_queue_free( static void gfx_widgets_msg_queue_free(
dispgfx_widget_t *p_dispwidget, dispgfx_widget_t *p_dispwidget,
menu_widget_msg_t *msg, bool touch_list) menu_widget_msg_t *msg)
{ {
size_t i;
uintptr_t tag = (uintptr_t)msg; uintptr_t tag = (uintptr_t)msg;
if (msg->task_ptr) if (msg->task_ptr)
@ -532,31 +533,35 @@ static void gfx_widgets_msg_queue_free(
if (msg->msg_new) if (msg->msg_new)
free(msg->msg_new); free(msg->msg_new);
/* Remove it from the list */
if (touch_list)
{
file_list_free_userdata(p_dispwidget->current_msgs,
p_dispwidget->msg_queue_kill);
for (i = p_dispwidget->msg_queue_kill; i <
p_dispwidget->current_msgs->size-1; i++)
p_dispwidget->current_msgs->list[i] =
p_dispwidget->current_msgs->list[i+1];
p_dispwidget->current_msgs->size--;
}
p_dispwidget->widgets_moving = false; p_dispwidget->widgets_moving = false;
} }
static void gfx_widgets_msg_queue_kill_end(void *userdata) static void gfx_widgets_msg_queue_kill_end(void *userdata)
{ {
dispgfx_widget_t *p_dispwidget = (dispgfx_widget_t*)dispwidget_get_ptr(); dispgfx_widget_t *p_dispwidget = (dispgfx_widget_t*)dispwidget_get_ptr();
menu_widget_msg_t *msg = (menu_widget_msg_t*) menu_widget_msg_t* msg;
p_dispwidget->current_msgs->list[p_dispwidget->msg_queue_kill].userdata; unsigned i;
SLOCK_LOCK(p_dispwidget->current_msgs_lock);
msg = p_dispwidget->current_msgs[p_dispwidget->msg_queue_kill];
if (msg) if (msg)
gfx_widgets_msg_queue_free(p_dispwidget, msg, true); {
/* Remove it from the list */
for (i = p_dispwidget->msg_queue_kill; i < p_dispwidget->current_msgs_size - 1; i++)
p_dispwidget->current_msgs[i] = p_dispwidget->current_msgs[i + 1];
p_dispwidget->current_msgs_size--;
p_dispwidget->current_msgs[p_dispwidget->current_msgs_size] = NULL;
/* clean up the item */
gfx_widgets_msg_queue_free(p_dispwidget, msg);
/* free the associated memory */
free(msg);
}
SLOCK_UNLOCK(p_dispwidget->current_msgs_lock);
} }
static void gfx_widgets_msg_queue_kill( static void gfx_widgets_msg_queue_kill(
@ -564,8 +569,7 @@ static void gfx_widgets_msg_queue_kill(
unsigned idx) unsigned idx)
{ {
gfx_animation_ctx_entry_t entry; gfx_animation_ctx_entry_t entry;
menu_widget_msg_t *msg = (menu_widget_msg_t*) menu_widget_msg_t *msg = p_dispwidget->current_msgs[idx];
p_dispwidget->current_msgs->list[idx].userdata;
if (!msg) if (!msg)
return; return;
@ -595,7 +599,7 @@ static void gfx_widgets_msg_queue_kill(
gfx_animation_push(&entry); gfx_animation_push(&entry);
/* Move all messages back to their correct position */ /* Move all messages back to their correct position */
if (p_dispwidget->current_msgs->size != 0) if (p_dispwidget->current_msgs_size != 0)
gfx_widgets_msg_queue_move(p_dispwidget); gfx_widgets_msg_queue_move(p_dispwidget);
} }
@ -848,68 +852,67 @@ void gfx_widgets_iterate(
/* Consume one message if available */ /* Consume one message if available */
if ((fifo_read_avail(p_dispwidget->msg_queue) > 0) if ((fifo_read_avail(p_dispwidget->msg_queue) > 0)
&& !p_dispwidget->widgets_moving && !p_dispwidget->widgets_moving
&& (p_dispwidget->current_msgs->size < MSG_QUEUE_ONSCREEN_MAX)) && (p_dispwidget->current_msgs_size < ARRAY_SIZE(p_dispwidget->current_msgs)))
{ {
menu_widget_msg_t *msg_widget; menu_widget_msg_t *msg_widget = NULL;
fifo_read(p_dispwidget->msg_queue, &msg_widget, sizeof(msg_widget)); SLOCK_LOCK(p_dispwidget->current_msgs_lock);
/* Task messages always appear from the bottom of the screen */ if (p_dispwidget->current_msgs_size < ARRAY_SIZE(p_dispwidget->current_msgs))
if (p_dispwidget->msg_queue_tasks_count == 0 || msg_widget->task_ptr)
{ {
file_list_append(p_dispwidget->current_msgs, if (fifo_read_avail(p_dispwidget->msg_queue) > 0)
NULL, fifo_read(p_dispwidget->msg_queue, &msg_widget, sizeof(msg_widget));
NULL,
0,
0,
0
);
file_list_set_userdata(p_dispwidget->current_msgs, if (msg_widget)
p_dispwidget->current_msgs->size-1, msg_widget); {
} /* Task messages always appear from the bottom of the screen, append it */
/* Regular messages are always above tasks */ if (p_dispwidget->msg_queue_tasks_count == 0 || msg_widget->task_ptr)
else {
{ p_dispwidget->current_msgs[p_dispwidget->current_msgs_size] = msg_widget;
unsigned idx = (unsigned)(p_dispwidget->current_msgs->size - }
p_dispwidget->msg_queue_tasks_count); /* Regular messages are always above tasks, make room and insert it */
file_list_insert(p_dispwidget->current_msgs, else
NULL, {
NULL, unsigned idx = (unsigned)(p_dispwidget->current_msgs_size -
0, p_dispwidget->msg_queue_tasks_count);
0, for (i = p_dispwidget->current_msgs_size; i > idx; i--)
0, p_dispwidget->current_msgs[i] = p_dispwidget->current_msgs[i - 1];
idx p_dispwidget->current_msgs[idx] = msg_widget;
); }
file_list_set_userdata(p_dispwidget->current_msgs, idx, msg_widget); p_dispwidget->current_msgs_size++;
}
} }
/* Start expiration timer if not associated to a task */ SLOCK_UNLOCK(p_dispwidget->current_msgs_lock);
if (!msg_widget->task_ptr)
if (msg_widget)
{ {
if (!msg_widget->expiration_timer_started) /* Start expiration timer if not associated to a task */
gfx_widgets_start_msg_expiration_timer( if (!msg_widget->task_ptr)
msg_widget, MSG_QUEUE_ANIMATION_DURATION * 2 {
if (!msg_widget->expiration_timer_started)
gfx_widgets_start_msg_expiration_timer(
msg_widget, MSG_QUEUE_ANIMATION_DURATION * 2
+ msg_widget->duration); + msg_widget->duration);
} }
/* Else, start hourglass animation timer */ /* Else, start hourglass animation timer */
else else
{ {
p_dispwidget->msg_queue_tasks_count++; p_dispwidget->msg_queue_tasks_count++;
gfx_widgets_hourglass_end(msg_widget); gfx_widgets_hourglass_end(msg_widget);
} }
if (p_dispwidget->current_msgs->size != 0) if (p_dispwidget->current_msgs_size != 0)
gfx_widgets_msg_queue_move(p_dispwidget); gfx_widgets_msg_queue_move(p_dispwidget);
}
} }
/* Kill first expired message */ /* Kill first expired message */
/* Start expiration timer of dead tasks */ /* Start expiration timer of dead tasks */
for (i = 0; i < p_dispwidget->current_msgs->size ; i++) for (i = 0; i < p_dispwidget->current_msgs_size; i++)
{ {
menu_widget_msg_t *msg_widget = (menu_widget_msg_t*) menu_widget_msg_t *msg_widget = p_dispwidget->current_msgs[i];
p_dispwidget->current_msgs->list[i].userdata;
if (!msg_widget) if (!msg_widget)
continue; continue;
@ -1748,24 +1751,30 @@ void gfx_widgets_frame(void *data)
} }
/* Draw all messages */ /* Draw all messages */
for (i = 0; i < p_dispwidget->current_msgs->size; i++) if (p_dispwidget->current_msgs_size)
{ {
menu_widget_msg_t *msg = (menu_widget_msg_t*) SLOCK_LOCK(p_dispwidget->current_msgs_lock);
p_dispwidget->current_msgs->list[i].userdata;
if (!msg) for (i = 0; i < p_dispwidget->current_msgs_size; i++)
continue; {
menu_widget_msg_t* msg = p_dispwidget->current_msgs[i];
if (msg->task_ptr) if (!msg)
gfx_widgets_draw_task_msg( continue;
if (msg->task_ptr)
gfx_widgets_draw_task_msg(
p_dispwidget, p_dispwidget,
msg, userdata, msg, userdata,
video_width, video_height); video_width, video_height);
else else
gfx_widgets_draw_regular_msg( gfx_widgets_draw_regular_msg(
p_dispwidget, p_dispwidget,
msg, userdata, msg, userdata,
video_width, video_height); video_width, video_height);
}
SLOCK_UNLOCK(p_dispwidget->current_msgs_lock);
} }
#ifdef HAVE_MENU #ifdef HAVE_MENU
@ -1831,15 +1840,12 @@ bool gfx_widgets_init(uintptr_t widgets_active_ptr,
if (!p_dispwidget->msg_queue) if (!p_dispwidget->msg_queue)
goto error; goto error;
p_dispwidget->current_msgs = (file_list_t*) memset(&p_dispwidget->current_msgs[0], 0, sizeof(p_dispwidget->current_msgs));
calloc(1, sizeof(file_list_t)); p_dispwidget->current_msgs_size = 0;
if (!p_dispwidget->current_msgs) #ifdef HAVE_THREADS
goto error; p_dispwidget->current_msgs_lock = slock_new();
#endif
if (!file_list_reserve(p_dispwidget->current_msgs,
MSG_QUEUE_ONSCREEN_MAX))
goto error;
p_dispwidget->widgets_inited = true; p_dispwidget->widgets_inited = true;
} }
@ -2265,7 +2271,7 @@ static void gfx_widgets_free(dispgfx_widget_t *p_dispwidget)
* will generate heap-use-after-free errors */ * will generate heap-use-after-free errors */
msg_widget->task_ptr = NULL; msg_widget->task_ptr = NULL;
gfx_widgets_msg_queue_free(p_dispwidget, msg_widget, false); gfx_widgets_msg_queue_free(p_dispwidget, msg_widget);
free(msg_widget); free(msg_widget);
} }
@ -2274,28 +2280,33 @@ static void gfx_widgets_free(dispgfx_widget_t *p_dispwidget)
p_dispwidget->msg_queue = NULL; p_dispwidget->msg_queue = NULL;
/* Purge everything from the list */ /* Purge everything from the list */
if (p_dispwidget->current_msgs) SLOCK_LOCK(p_dispwidget->current_msgs_lock);
p_dispwidget->current_msgs_size = 0;
for (i = 0; i < ARRAY_SIZE(p_dispwidget->current_msgs); i++)
{ {
for (i = 0; i < p_dispwidget->current_msgs->size; i++) menu_widget_msg_t *msg = p_dispwidget->current_msgs[i];
{ if (!msg)
menu_widget_msg_t *msg = (menu_widget_msg_t*) continue;
p_dispwidget->current_msgs->list[i].userdata;
/* Note: gfx_widgets_free() is only called when /* Note: gfx_widgets_free() is only called when
* main_exit() is invoked. At this stage, we cannot * main_exit() is invoked. At this stage, we cannot
* guarantee that any task pointers are valid (the * guarantee that any task pointers are valid (the
* task may have been free()'d, but we can't know * task may have been free()'d, but we can't know
* that here) - so all we can do is unset the task * that here) - so all we can do is unset the task
* pointer associated with each message * pointer associated with each message
* > If we don't do this, gfx_widgets_msg_queue_free() * > If we don't do this, gfx_widgets_msg_queue_free()
* will generate heap-use-after-free errors */ * will generate heap-use-after-free errors */
msg->task_ptr = NULL; msg->task_ptr = NULL;
gfx_widgets_msg_queue_free(p_dispwidget, msg, false); gfx_widgets_msg_queue_free(p_dispwidget, msg);
}
file_list_free(p_dispwidget->current_msgs);
} }
p_dispwidget->current_msgs = NULL; SLOCK_UNLOCK(p_dispwidget->current_msgs_lock);
#ifdef HAVE_THREADS
slock_free(p_dispwidget->current_msgs_lock);
p_dispwidget->current_msgs_lock = NULL;
#endif
p_dispwidget->msg_queue_tasks_count = 0; p_dispwidget->msg_queue_tasks_count = 0;
@ -2311,6 +2322,10 @@ static void gfx_widgets_free(dispgfx_widget_t *p_dispwidget)
SLOCK_UNLOCK(p_dispwidget->cheevo_popup_queue_lock); SLOCK_UNLOCK(p_dispwidget->cheevo_popup_queue_lock);
} }
#ifdef HAVE_THREADS
slock_free(p_dispwidget->cheevo_popup_queue_lock);
p_dispwidget->cheevo_popup_queue_lock = NULL;
#endif
#endif #endif
/* Font */ /* Font */

View File

@ -24,7 +24,6 @@
#include <queues/task_queue.h> #include <queues/task_queue.h>
#include <queues/message_queue.h> #include <queues/message_queue.h>
#include <queues/fifo_queue.h> #include <queues/fifo_queue.h>
#include <lists/file_list.h>
#ifdef HAVE_THREADS #ifdef HAVE_THREADS
#include <rthreads/rthreads.h> #include <rthreads/rthreads.h>
@ -246,11 +245,15 @@ typedef struct dispgfx_widget
/* Achievement notification */ /* Achievement notification */
cheevo_popup cheevo_popup_queue[CHEEVO_QUEUE_SIZE]; cheevo_popup cheevo_popup_queue[CHEEVO_QUEUE_SIZE];
gfx_timer_t cheevo_timer; gfx_timer_t cheevo_timer;
#ifdef HAVE_THREADS
slock_t* cheevo_popup_queue_lock;
#endif
#endif #endif
fifo_buffer_t *msg_queue; fifo_buffer_t *msg_queue;
file_list_t *current_msgs; menu_widget_msg_t* current_msgs[MSG_QUEUE_ONSCREEN_MAX];
#if defined(HAVE_CHEEVOS) && defined(HAVE_THREADS) size_t current_msgs_size;
slock_t* cheevo_popup_queue_lock; #ifdef HAVE_THREADS
slock_t* current_msgs_lock;
#endif #endif
} dispgfx_widget_t; } dispgfx_widget_t;