xwayland: Prevent nested xwl_present_for_each_frame_callback calls

It could happen with the following call path:

frame_callback
 xwl_present_frame_callback
  xwl_present_msc_bump
   xwl_present_execute
    xwl_present_flip
     xwl_window_create_frame_callback

The nested loop called xwl_present_reset_timer, which may end up calling
xorg_list_del for the entry after the one frame_callback started the
chain for. This resulted in the outer loop never terminating, because
its next element wasn't hooked up to the list anymore.

We avoid this by calling xwl_present_reset_timer as needed in
frame_callback, and bailing from xwl_window_create_frame_callback if it
was called from the former.

We also catch nested calls and FatalError if they ever happen again due
to another bug.

v2:
* Leave xwl_present_reset_timer call in xwl_present_frame_callback,
  needed if xwl_present_msc_bump didn't hook up the window to the frame
  callback list again.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1442
This commit is contained in:
Michel Dänzer 2023-03-10 12:39:30 +01:00 committed by Michel Dänzer
parent 4d1cd7cdc2
commit 754d6b6dd0
3 changed files with 30 additions and 2 deletions

View File

@ -89,16 +89,31 @@ xwl_present_event_from_id(uint64_t event_id)
return (struct xwl_present_event*)(uintptr_t)event_id; return (struct xwl_present_event*)(uintptr_t)event_id;
} }
static Bool entered_for_each_frame_callback;
Bool
xwl_present_entered_for_each_frame_callback(void)
{
return entered_for_each_frame_callback;
}
void void
xwl_present_for_each_frame_callback(struct xwl_window *xwl_window, xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
void iter_func(struct xwl_present_window *)) void iter_func(struct xwl_present_window *))
{ {
struct xwl_present_window *xwl_present_window, *tmp; struct xwl_present_window *xwl_present_window, *tmp;
if (entered_for_each_frame_callback)
FatalError("Nested xwl_present_for_each_frame_callback call");
entered_for_each_frame_callback = TRUE;
xorg_list_for_each_entry_safe(xwl_present_window, tmp, xorg_list_for_each_entry_safe(xwl_present_window, tmp,
&xwl_window->frame_callback_list, &xwl_window->frame_callback_list,
frame_callback_list) frame_callback_list)
iter_func(xwl_present_window); iter_func(xwl_present_window);
entered_for_each_frame_callback = FALSE;
} }
static void static void

View File

@ -61,6 +61,7 @@ struct xwl_present_event {
PixmapPtr pixmap; PixmapPtr pixmap;
}; };
Bool xwl_present_entered_for_each_frame_callback(void);
void xwl_present_for_each_frame_callback(struct xwl_window *xwl_window, void xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
void iter_func(struct xwl_present_window *)); void iter_func(struct xwl_present_window *));
void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window); void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window);

View File

@ -1226,8 +1226,16 @@ frame_callback(void *data,
xwl_window->frame_callback = NULL; xwl_window->frame_callback = NULL;
#ifdef GLAMOR_HAS_GBM #ifdef GLAMOR_HAS_GBM
if (xwl_window->xwl_screen->present) if (xwl_window->xwl_screen->present) {
xwl_present_for_each_frame_callback(xwl_window, xwl_present_frame_callback); xwl_present_for_each_frame_callback(xwl_window, xwl_present_frame_callback);
/* If xwl_window_create_frame_callback was called from
* xwl_present_frame_callback, need to make sure all fallback timers
* are adjusted correspondingly.
*/
if (xwl_window->frame_callback)
xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
}
#endif #endif
} }
@ -1243,7 +1251,11 @@ xwl_window_create_frame_callback(struct xwl_window *xwl_window)
xwl_window); xwl_window);
#ifdef GLAMOR_HAS_GBM #ifdef GLAMOR_HAS_GBM
if (xwl_window->xwl_screen->present) /* If we get called from frame_callback, it will take care of calling
* xwl_present_reset_timer.
*/
if (xwl_window->xwl_screen->present &&
!xwl_present_entered_for_each_frame_callback())
xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer); xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
#endif #endif
} }