1
0
Fork 0

frame: Handle reparent failure avoiding duplicate stack windows instances

In some cases, reparenting a window with its frame may fail; this seems
to happen especially during initialization of a window that may be
unmapped and re-mapped quickly and multiple times.

If this happens, we're never going to receive a remove event on the
stack tracker and so we may end up adding it twice to the list of the
windows to synchronize with the compositor, breaking its assumption that
the stack list is unique, and eventually leading to a crash because we
do not end up removing all the instances of a window on its destruction.

In particular we may end up in this situation:

  Syncing Window 10485927: 0x555558863540 (actual xid is 10485927),
    user time is 10485928 frame is 0x5555588715c0, frame xid 6291591
  Syncing Window 14680081: 0x5555588664b0 (actual xid is 14680081),
    user time is 14680082 frame is 0x555558871d80, frame xid 6291595
  Syncing Window 6291460: 0x55555796dc80 (actual xid is 10485763),
    user time is 10485764 frame is 0x555557a6f630, frame xid 6291460
  Syncing Window 6291465: 0x555557a68af0 (actual xid is 14680067),
    user time is 14680068 frame is 0x555557a73e80, frame xid 6291465
  Syncing Window 6291509: 0x555557f9d830 (actual xid is 8388623),
    user time is 0 frame is 0x555557fac780, frame xid 6291509
  Syncing Window 6291586: 0x5555586e1690 (actual xid is 4194363),
    user time is 0 frame is 0x55555886e550, frame xid 6291586
  Syncing Window 6291591: 0x555558863540 (actual xid is 10485927),
    user time is 10485928 frame is 0x5555588715c0, frame xid 6291591

Where the same meta window 0x555558863540 is added twice because that's
both mapped by the window itself (10485927) and by its frame (6291591).

This happens because for historical reasons the xids hash table managed
by the x11-display maps both the X11 windows, their frames and their
user time windows as the meta-window, and so if we don't filter out them
properly we end up duplicating the entries in the compositor list.

Such duplicates finally end up making mutter to crash in
meta_compositor_sync_stack() because we could end up trying to access
to an invalid window, given its actor has been destroyed but not all the
instances have been removed from the compositor windows list:

  0x00007ffff71059 in meta_compositor_sync_stack (compositor=0x555555b8,
  stack=0x555558701b80) at ../../mutter/src/compositor/compositor.c:773
  773	          if ((old_window->hidden || old_window->unmanaging) &&
  (gdb) print old_window
  $1 = (MetaWindow *) 0x0

So, in order to prevent this, check that XReparentWindow does not fail,
and in case of failure, reset the window state to the one it had before
we failed and more importantly, remove the association between the frame
X11 window and the MetaWindow, since this is not true anymore and so
that at the next stack synchronization there won't be any meta window
associated to that frame XID (unless there aren't further stack changes
impacting on that). Without this we would have instead waited for the
remove event that we predicted, but that could never happen because no
ReparentNotify is emitted in such case.
This commit is contained in:
Marco Trevisan (Treviño) 2024-05-04 04:17:02 +02:00
parent bd799b6b36
commit 1e02c06298

View file

@ -35,6 +35,7 @@
#include "x11/window-props.h"
#include <X11/Xatom.h>
#include <X11/Xlib.h>
#include <X11/extensions/shape.h>
#define EVENT_MASK (SubstructureRedirectMask | \
@ -132,6 +133,22 @@ meta_window_x11_set_frame_xwindow (MetaWindow *window,
frame->child_x,
frame->child_y);
if (mtk_x11_error_trap_pop_with_return (x11_display->xdisplay))
{
/* Reparent failed, so we need to restore the window state and
* remove the association of the frame xwindow with the window
* otherwise the we'll associate the events for this frame to the
* logical window and we may end up duplicating it when the stack
* will be synchronized with the compositor leading to the same logical
* window appearing multiple times in the stack, which is not expected.
*/
meta_topic (META_DEBUG_WINDOW_STATE, "Failed to set %lu as %s frame",
frame->xwindow, window->desc);
meta_x11_display_unregister_x_window (x11_display, frame->xwindow);
return;
}
if (window->mapped)
{
window->mapped = FALSE; /* the reparent will unmap the window,
@ -146,8 +163,6 @@ meta_window_x11_set_frame_xwindow (MetaWindow *window,
window->frame = g_steal_pointer (&frame);
window->reparents_pending += 1;
/* FIXME handle this error */
mtk_x11_error_trap_pop (x11_display->xdisplay);
/* Ensure focus is restored after the unmap/map events triggered
* by XReparentWindow().