race condition when closing popups

[dkondor]

Hi,

copying my original bug report for Wayfire (WayfireWM/wayfire#772), since this is probably an issue with wlroots.

Steps to reproduce:

1. Build and run cairo-dock from this branch: https://github.com/dkondor/cairo-dock-core/tree/wayland_egl
2. Start wayfire and cairo-dock (cairo-dock -o -L)
3. Start any app (gnome-calculator in my example)
4. Minimize it
5. Right click on the icon
6. From the right click menu, click on the magnifying glass icon (see screenshot)

Expected result: app is restored and activated
Actual result: wayfire crashes (see below for stacktrace)

In this case, cairo-dock is using the activate request of wlr-foreign-toplevel-management.

[dkondor]

Stacktrace from wayfire:

wayfire: ../types/xdg_shell/wlr_xdg_surface.c:364: void handle_xdg_surface_commit(struct wlr_surface *): Assertion `false' failed.
EE 08-10-20 12:38:37.817 - [src/main.cpp:244] Fatal error: Fatal error(SIGABRT)
EE 08-10-20 12:38:37.865 - #1  signal_handler(int) ../src/main.cpp:245
EE 08-10-20 12:38:38.040 - #2  killpg ??:?
EE 08-10-20 12:38:38.178 - #3  __GI_raise ../sysdeps/unix/sysv/linux/raise.c:51 (discriminator 3)
EE 08-10-20 12:38:38.317 - #4  __GI_abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:81
EE 08-10-20 12:38:38.454 - #5  __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:89
EE 08-10-20 12:38:38.629 - #6  __assert_fail ??:?
EE 08-10-20 12:38:38.641 - #7  handle_xdg_surface_commit ../types/xdg_shell/wlr_xdg_surface.c:366
EE 08-10-20 12:38:38.655 - #8  surface_commit_pending ../types/wlr_surface.c:430
EE 08-10-20 12:38:38.669 - #9  surface_commit ../types/wlr_surface.c:502
EE 08-10-20 12:38:38.677 - #10 ffi_call_unix64 ??:?
EE 08-10-20 12:38:38.684 - #11 ffi_call ??:?
EE 08-10-20 12:38:38.694 - #12 wl_closure_invoke /home/dkondor/program/wayland/src/connection.c:1020
EE 08-10-20 12:38:38.703 - #13 wl_client_connection_data /home/dkondor/program/wayland/src/wayland-server.c:432
EE 08-10-20 12:38:38.711 - #14 wl_event_loop_dispatch /home/dkondor/program/wayland/src/event-loop.c:1027
EE 08-10-20 12:38:38.721 - #15 wl_display_run /home/dkondor/program/wayland/src/wayland-server.c:1349
EE 08-10-20 12:38:38.750 - #16 main ../src/main.cpp:406
EE 08-10-20 12:38:38.885 - #17 __libc_start_main ../csu/libc-start.c:344
EE 08-10-20 12:38:38.924 - #18 ?? ??:0
EE 08-10-20 12:38:38.927 - [EGL] command: eglMakeCurrent, error: EGL_BAD_DISPLAY (0x3008), message: "eglMakeCurrent"
EE 08-10-20 12:38:38.927 - [render/egl.c:431] eglMakeCurrent failed
wayfire: ../render/gles2/renderer.c:35: struct wlr_gles2_renderer *gles2_get_renderer_in_context(struct wlr_renderer *): Assertion `wlr_egl_is_current(renderer->egl)' failed.
EE 08-10-20 12:38:38.927 - [src/main.cpp:244] Fatal error: Fatal error(SIGABRT)

[dkondor]

I've run both wayfire and cairo-dock with WAYLAND_DEBUG=1, and this is the part of the output I think is relevant:

For wayfire:

[4140950.033] zwlr_foreign_toplevel_handle_v1@4278190080.activate(wl_seat@11)
...
[4140953.836]  -> xdg_popup@40.popup_done()
[4140962.837] wl_surface@45.commit()

For cairo-dock

[4140949.599]  -> zwlr_foreign_toplevel_handle_v1@4278190080.activate(wl_seat@11)
...
[4140955.739]  -> wl_surface@45.commit()
[4140956.080] xdg_popup@40.popup_done()
[4140956.100]  -> xdg_popup@40.destroy()
[4140956.114]  -> xdg_surface@41.destroy()
[4140956.140]  -> wl_surface@45.destroy()

Here, as far as I can tell, wl_surface@45, xdg_surface@41 and xdg_popup@40 belong to the menu. From wayfire's perspective, it sends the popup_done signal in response to the activate request, but receives a commit request for the popup's wl_surface after it. From cairo-dock's perspective, the popup_done event does not arrive until after the commit request is sent.

[emersion]

The crash seems like a duplicate of #2056

[dkondor]

I've created a bit contrived example that reliably crashes Wayfire for me:
https://github.com/dkondor/wlroots/tree/issue-2432/issue-2432

The crash triggered by this code

(the render_frame() function does some basic rendering and commits the given wl_surface)

Output with WAYLAND_DEBUG in the client (after clicking on the popup):

[4071094.721] wl_pointer@3.frame()
[4072220.989] wl_pointer@3.button(27, 3640342, 272, 1)

Sending activate, this should close the popup
[4072221.063]  -> zwlr_foreign_toplevel_handle_v1@4278190081.activate(wl_seat@7)

Commiting the popup

[4072226.363]  -> wl_shm@4.create_pool(new id wl_shm_pool@21, fd 5, 1440000)
[4072226.537]  -> wl_shm_pool@21.create_buffer(new id wl_buffer@19, 0, 300, 300, 1200, 0)
[4072226.768]  -> wl_shm_pool@21.destroy()
[4072228.081]  -> wl_surface@15.attach(wl_buffer@19, 0, 0)
[4072228.167]  -> wl_surface@15.commit()
[4072228.281] wl_pointer@3.frame()
[4072228.385] zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072228.428] zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072228.456] zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072228.478] zwlr_foreign_toplevel_handle_v1@4278190081.done()
[4072228.498] xdg_popup@17.popup_done()

Destroying popup

[4072228.530]  -> xdg_popup@17.destroy()

Output from Wayfire:

[4071094.513]  -> wl_pointer@3.frame()
[4072220.761]  -> wl_pointer@3.button(27, 3640342, 272, 1)
[4072220.888]  -> wl_pointer@3.frame()
[4072221.135] zwlr_foreign_toplevel_handle_v1@4278190081.activate(wl_seat@7)
[4072221.344]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072221.382]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072221.417]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072222.461]  -> zwlr_foreign_toplevel_handle_v1@4278190081.done()
[4072222.501]  -> xdg_popup@17.popup_done()
[4072222.747]  -> wl_pointer@3.leave(28, wl_surface@15)
[4072222.786]  -> wl_pointer@3.frame()
[4072222.799]  -> wl_pointer@3.enter(29, wl_surface@8, 259.000000, 253.000000)
[4072222.876]  -> wl_pointer@3.frame()
[4072223.070]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.106]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.138]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.330]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.356]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.370]  -> zwlr_foreign_toplevel_handle_v1@4278190081.state(array)
[4072223.497]  -> zwlr_foreign_toplevel_handle_v1@4278190081.done()
[4072228.393] wl_shm@4.create_pool(new id wl_shm_pool@21, fd 26, 1440000)
[4072228.505] wl_shm_pool@21.create_buffer(new id wl_buffer@19, 0, 300, 300, 1200, 0)
[4072228.597] wl_shm_pool@21.destroy()
[4072228.619]  -> wl_display@1.delete_id(21)
[4072228.651] wl_surface@15.attach(wl_buffer@19, 0, 0)
[4072228.700] wl_surface@15.commit()
[4072229.676]  -> wl_buffer@19.release()

Crash happens after this, giving this stacktrace:

wayfire: ../types/xdg_shell/wlr_xdg_surface.c:364: void handle_xdg_surface_commit(struct wlr_surface *): Assertion `false' failed.
EE 15-10-20 00:26:38.332 - [src/main.cpp:233] Fatal error: Fatal error(SIGABRT)
EE 15-10-20 00:26:38.381 - #1  signal_handler(int) ../src/main.cpp:234
EE 15-10-20 00:26:38.599 - #2  killpg ??:?
EE 15-10-20 00:26:38.768 - #3  __GI_raise ../sysdeps/unix/sysv/linux/raise.c:51 (discriminator 3)
EE 15-10-20 00:26:38.932 - #4  __GI_abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:81
EE 15-10-20 00:26:39.098 - #5  __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:89
EE 15-10-20 00:26:39.310 - #6  __assert_fail ??:?
EE 15-10-20 00:26:39.330 - #7  handle_xdg_surface_commit ../types/xdg_shell/wlr_xdg_surface.c:366
EE 15-10-20 00:26:39.355 - #8  surface_commit_pending ../types/wlr_surface.c:430
EE 15-10-20 00:26:39.378 - #9  surface_commit ../types/wlr_surface.c:502
EE 15-10-20 00:26:39.391 - #10 ffi_call_unix64 ??:?
EE 15-10-20 00:26:39.409 - #11 ffi_call ??:?
EE 15-10-20 00:26:39.426 - #12 wl_closure_invoke /home/dkondor/program/wayland/src/connection.c:1020
EE 15-10-20 00:26:39.445 - #13 wl_client_connection_data /home/dkondor/program/wayland/src/wayland-server.c:432
EE 15-10-20 00:26:39.461 - #14 wl_event_loop_dispatch /home/dkondor/program/wayland/src/event-loop.c:1027
EE 15-10-20 00:26:39.480 - #15 wl_display_run /home/dkondor/program/wayland/src/wayland-server.c:1349
EE 15-10-20 00:26:39.526 - #16 main ../src/main.cpp:393
EE 15-10-20 00:26:39.693 - #17 __libc_start_main ../csu/libc-start.c:344
EE 15-10-20 00:26:39.742 - #18 ?? ??:0
EE 15-10-20 00:26:39.746 - [src/main.cpp:233] Fatal error: Segmentation fault
EE 15-10-20 00:26:39.799 - #1  signal_handler(int) ../src/main.cpp:234
EE 15-10-20 00:26:40.018 - #2  killpg ??:?
EE 15-10-20 00:26:40.103 - #3  std::_Rb_tree<wf_binding_type, std::pair<wf_binding_type const, std::vector<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> >, std::allocator<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> > > > >, std::_Select1st<std::pair<wf_binding_type const, std::vector<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> >, std::allocator<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> > > > > >, std::less<wf_binding_type>, std::allocator<std::pair<wf_binding_type const, std::vector<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> >, std::allocator<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> > > > > > >::begin() ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:984
EE 15-10-20 00:26:40.189 - #4  std::map<wf_binding_type, std::vector<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> >, std::allocator<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> > > >, std::less<wf_binding_type>, std::allocator<std::pair<wf_binding_type const, std::vector<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> >, std::allocator<std::unique_ptr<wf::binding_t, std::default_delete<wf::binding_t> > > > > > >::begin() ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_map.h:355
EE 15-10-20 00:26:40.269 - #5  input_manager::rem_binding(std::function<bool (wf::binding_t*)>) ../src/core/seat/input-manager.cpp:384
EE 15-10-20 00:26:40.353 - #6  input_manager::rem_binding(void*) ../src/core/seat/input-manager.cpp:408
EE 15-10-20 00:26:40.431 - #7  wf::output_t::rem_binding(void*) ../src/output/output.cpp:497
EE 15-10-20 00:26:40.453 - #8  wayfire_zoom_screen::fini() ../plugins/single_plugins/zoom.cpp:114
EE 15-10-20 00:26:40.533 - #9  plugin_manager::destroy_plugin(std::unique_ptr<wf::plugin_interface_t, std::default_delete<wf::plugin_interface_t> >&) ../src/output/plugin-loader.cpp:82
EE 15-10-20 00:26:40.610 - #10 plugin_manager::deinit_plugins(bool) ../src/output/plugin-loader.cpp:48
EE 15-10-20 00:26:40.692 - #11 ~plugin_manager ../src/output/plugin-loader.cpp:65
EE 15-10-20 00:26:40.770 - #12 std::default_delete<plugin_manager>::operator()(plugin_manager*) const ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:81
EE 15-10-20 00:26:40.845 - #13 ~unique_ptr ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:277
EE 15-10-20 00:26:40.919 - #14 ~output_impl_t ../src/output/output.cpp:99
EE 15-10-20 00:26:40.995 - #15 ~output_impl_t ../src/output/output.cpp:99
EE 15-10-20 00:26:41.082 - #16 std::default_delete<wf::output_impl_t>::operator()(wf::output_impl_t*) const ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:82
EE 15-10-20 00:26:41.171 - #17 ~unique_ptr ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:277
EE 15-10-20 00:26:41.261 - #18 ~output_layout_output_t ../src/core/output-layout.cpp:294
EE 15-10-20 00:26:41.350 - #19 std::default_delete<wf::output_layout_output_t>::operator()(wf::output_layout_output_t*) const ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:81
EE 15-10-20 00:26:41.434 - #20 ~unique_ptr ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:277
EE 15-10-20 00:26:41.524 - #21 ~pair ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_pair.h:193
EE 15-10-20 00:26:41.613 - #22 void __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > >::destroy<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >(std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > >*) ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/ext/new_allocator.h:140
EE 15-10-20 00:26:41.699 - #23 void std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > > >::destroy<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >(std::allocator<std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > >&, std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > >*) ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/alloc_traits.h:487
EE 15-10-20 00:26:41.789 - #24 std::_Rb_tree<wlr_output*, std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > >, std::_Select1st<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >, std::less<wlr_output*>, std::allocator<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > >::_M_destroy_node(std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >*) ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:650
EE 15-10-20 00:26:41.878 - #25 std::_Rb_tree<wlr_output*, std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > >, std::_Select1st<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >, std::less<wlr_output*>, std::allocator<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > >::_M_drop_node(std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >*) ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:659
EE 15-10-20 00:26:41.965 - #26 std::_Rb_tree<wlr_output*, std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > >, std::_Select1st<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >, std::less<wlr_output*>, std::allocator<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > > >::_M_erase(std::_Rb_tree_node<std::pair<wlr_output* const, std::unique_ptr<wf::output_layout_output_t, std::default_delete<wf::output_layout_output_t> > > >*) ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:1882
EE 15-10-20 00:26:42.056 - #27 ~_Rb_tree ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:972
EE 15-10-20 00:26:42.140 - #28 ~map ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_map.h:300
EE 15-10-20 00:26:42.226 - #29 ~impl ../src/core/output-layout.cpp:1027
EE 15-10-20 00:26:42.314 - #30 std::default_delete<wf::output_layout_t::impl>::operator()(wf::output_layout_t::impl*) const ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:81
EE 15-10-20 00:26:42.403 - #31 ~unique_ptr ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:277
EE 15-10-20 00:26:42.489 - #32 ~output_layout_t ../src/core/output-layout.cpp:1653
EE 15-10-20 00:26:42.574 - #33 ~output_layout_t ../src/core/output-layout.cpp:1653
EE 15-10-20 00:26:42.649 - #34 std::default_delete<wf::output_layout_t>::operator()(wf::output_layout_t*) const ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:82
EE 15-10-20 00:26:42.718 - #35 ~unique_ptr ../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/unique_ptr.h:277
EE 15-10-20 00:26:42.791 - #36 ~compositor_core_t ../src/core/core.cpp:771
EE 15-10-20 00:26:42.862 - #37 ~compositor_core_impl_t ../src/core/core.cpp:781
EE 15-10-20 00:26:43.031 - #38 __run_exit_handlers /build/glibc-2ORdQG/glibc-2.27/stdlib/exit.c:109
EE 15-10-20 00:26:43.250 - #39 exit ??:?
EE 15-10-20 00:26:43.292 - #40 signal_handler(int) ../src/main.cpp:236
EE 15-10-20 00:26:43.511 - #41 killpg ??:?
EE 15-10-20 00:26:43.680 - #42 __GI_raise ../sysdeps/unix/sysv/linux/raise.c:51 (discriminator 3)
EE 15-10-20 00:26:43.852 - #43 __GI_abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:81
EE 15-10-20 00:26:44.017 - #44 __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:89
EE 15-10-20 00:26:44.231 - #45 __assert_fail ??:?
EE 15-10-20 00:26:44.251 - #46 handle_xdg_surface_commit ../types/xdg_shell/wlr_xdg_surface.c:366
EE 15-10-20 00:26:44.272 - #47 surface_commit_pending ../types/wlr_surface.c:430
EE 15-10-20 00:26:44.297 - #48 surface_commit ../types/wlr_surface.c:502
EE 15-10-20 00:26:44.310 - #49 ffi_call_unix64 ??:?
EE 15-10-20 00:26:44.326 - #50 ffi_call ??:?
EE 15-10-20 00:26:44.344 - #51 wl_closure_invoke /home/dkondor/program/wayland/src/connection.c:1020
EE 15-10-20 00:26:44.362 - #52 wl_client_connection_data /home/dkondor/program/wayland/src/wayland-server.c:432
EE 15-10-20 00:26:44.377 - #53 wl_event_loop_dispatch /home/dkondor/program/wayland/src/event-loop.c:1027
EE 15-10-20 00:26:44.395 - #54 wl_display_run /home/dkondor/program/wayland/src/wayland-server.c:1349
EE 15-10-20 00:26:44.443 - #55 main ../src/main.cpp:393
EE 15-10-20 00:26:44.607 - #56 __libc_start_main ../csu/libc-start.c:344
EE 15-10-20 00:26:44.661 - #57 ?? ??:0

[ammen99]

Just a note, in the Wayfire stacktraces, the second stacktrace can be ignored, because it simply shows that the compositor fails to shutdown cleanly after handling the first SIGABRT (so only the first 18 frames are relevant).

[emersion]

Can you check whether #2453 helps?

[dkondor]

Can you check whether #2453 helps?

Just tested, it doesn't help.