Skip to content

Fix a crash in computeLayer()

Vlad Zahorodnii requested to merge work/zzag/fix-crash-in-compute-layer into master

On X11, the stack order of a window can be changed in multiple ways:

  • Opposite
  • TopIf
  • BottomIf
  • Above
  • Below

You would pass either of those modes plus maybe the above window id. For this crash, the relevant mode is Above.

Since the Workspace only has one restack function that places the given window right under the reference window, the Above stack mode implementation performs a quirk: it walks through the stack in order to find a window right above the reference window and pass it to the restack() function.

However, it could be that the window that wants to be restacked is already above the reference window, so that same window would be passed as the "under" window to the restack() function.

It's nothing but a miracle that we have not received major complaints about this issue until now.

The restack() function doesn't like window and under to be the same because of code like

unconstrained_stacking_order.removeAll(window);
unconstrained_stacking_order.insert(unconstrained_stacking_order.indexOf(under), window);

The removeAll() window would effectively remove both window and under from the unconstrained stack, which breaks the next line because the indexOf() would return -1, i.e.

unconstrained_stacking_order.insert(-1, window);

This is the reason why the unconstrained_stacking_order contains strange values such as 0xe.

In order to fix the crash, this change adds some code to short-circuit the restack() function if the passed in window and the reference window are the same. It would be great to clean up X11Window::restackWindow() and also add ergonomic restack functions in the Workspace, but this can be done later. The major blocker is lack of proper test coverage of X11 bits at the moment.

Last but not least, I would like to express my profound gratitude to Peter Strick for filing the crash report AND providing a VM image that helped massively with reproducing the crash and finally fixing it.

BUG: 491618

Merge request reports

Loading