Skip to content

layer-surface: Don't arrange surfaces / set focus on finalize

Guido Günther requested to merge guidog/phoc:stack-order into main

(N.B.: I've only tested this against !528 (merged) so far but should work the same way with 0.17 as it has the same guarantees)

This already happens on unmap and if we do it in finalize() (triggered via handle_destroy() we might end up in situations where some of the destroy handlers already ran but others (e.g. from PhocStackableLayerSurfce) didn't, leading to assertions like:


 #6  0x0000555555579a1a in phoc_output_get_layer_surfaces_for_layer (self=self@entry=0x5555561532a0 [PhocOutput], layer=layer@entry=ZWLR_LAYER_SHELL_V1_LAYER_TOP)
     at ../src/output.c:1325
 #7  0x000055555556df13 in layer_surface_at
     (output=output@entry=0x5555561532a0 [PhocOutput], layer=layer@entry=ZWLR_LAYER_SHELL_V1_LAYER_TOP, ox=100, oy=100, sx=sx@entry=0x7fffffffce30, sy=sy@entry=0x7fffffffce38) at ../src/desktop.c:205
 #8  0x000055555556fc17 in phoc_desktop_wlr_surface_at
     (desktop=0x5555560915a0 [PhocDesktop], lx=100, ly=100, sx=sx@entry=0x7fffffffce30, sy=sy@entry=0x7fffffffce38, view=view@entry=0x7fffffffce40) at ../src/desktop.c:279
 #9  0x0000555555592d6b in phoc_passthrough_cursor (self=self@entry=0x55555610ad00 [PhocCursor], time=168670331) at ../src/cursor.c:862
 #10 0x0000555555593282 in phoc_cursor_update_focus (self=0x55555610ad00 [PhocCursor]) at ../src/cursor.c:1196
 #11 0x0000555555582cb8 in phoc_seat_set_focus_layer (seat=0x55555610a200 [PhocSeat], layer=0x55555619d070) at ../src/seat.c:1592
 #12 0x00005555555748dd in phoc_layer_shell_update_focus () at ../src/layer-shell.c:371
 #13 0x000055555557382f in phoc_layer_surface_finalize (object=0x55555619b490 [PhocLayerSurface]) at ../src/layer-surface.c:362
 #14 0x00007ffff7be34ee in g_object_unref (_object=0x55555619b490) at ../../../gobject/gobject.c:4486
 #15 0x00007ffff76f4b4c in wl_signal_emit_mutable (signal=signal@entry=0x55555619b418, data=data@entry=0x55555619b350) at ../src/wayland-server.c:2314
 #16 0x00007ffff7f30ed3 in layer_surface_destroy (surface=0x55555619b350) at ../subprojects/wlroots/types/wlr_layer_shell_v1.c:52
 #17 0x00007ffff7f23f3a in surface_destroy_role_object (surface=0x55555619aed0) at ../subprojects/wlroots/types/wlr_compositor.c:919
 #18 surface_destroy_role_object (surface=0x55555619aed0) at ../subprojects/wlroots/types/wlr_compositor.c:913
 #19 surface_handle_resource_destroy (resource=<optimized out>) at ../subprojects/wlroots/types/wlr_compositor.c:727
 #20 0x00007ffff76f31d7 in remove_and_destroy_resource (element=0x55555616cf80, data=<optimized out>, flags=0) at ../src/wayland-server.c:757
 #21 0x00007ffff76f9c40 in for_each_helper (func=func@entry=0x7ffff76f3130 <remove_and_destroy_resource>, data=data@entry=0x0, entries=0x5555561bf7f0)
     at ../src/wayland-util.c:430
 #22 0x00007ffff76fa1f3 in wl_map_for_each (map=0x5555561bf7f0, func=0x7ffff76f3130 <remove_and_destroy_resource>, data=0x0) at ../src/wayland-util.c:444
 #23 0x00007ffff76f39e5 in wl_client_destroy (client=client@entry=0x5555561bf7c0) at ../src/wayland-server.c:961

The above happens when layer-surface's finalize() runs (triggering phoc_seat_set_focus_layer()) while stacked_layer_surface_handle_destroy() didn't run yet. Thus layer-surface is already gone from the output's list of known layer surfaces but can still be found in the list of stacks. So when iterating of the stacks and trying to lookup the surface on the output we hit the assertion.

An alternative solution would be to ignore the failed lookup but we rather want to keep it to check consistency.

An other alternative would be to run phoc_cursor_update_focus() in and idle callback but why so if we don't need it at all.

wlroots guarantees that a layer surface is unmapped before destroy so this should be safe to do.

Thanks to Sam Day for the nicer reproducer.

Closes: phosh#1161 (closed)

Signed-off-by: Guido Günther agx@sigxcpu.org

Edited by Guido Günther

Merge request reports

Loading