On Thu Sep 26 07:05:10 2024 +0000, Alexandros Frantzis wrote:
The core problem seems to be that in `wayland_win_data_update_wayland_state` `surface->role` is used to determine what's the *current* role for the surface. However `surface->role` is really tracking what role was given in the past to this surface and thus can be given again in the future (because surfaces cannot change roles). In this particular osu! case, we have a previously toplevel surface (so surface->role = toplevel) that is now role-less, but `wayland_surface_update_state_toplevel` is called on it, which then unconditionally tries to access the NULL xdg_toplevel and crashes the thread while holding the 'win_data_mutex` lock.
One fix is to add `if (!surface->xdg_toplevel) return;` at the beginning of `wayland_surface_update_state_toplevel()`. It depends on what we want `surface->role` to mean. If it is "possible role" (which seems to match how other parts of the code are implemented) then this should be OK.
Alternatively, perhaps a more explicit arrangement `surface->role` vs `surface->possible_role` would make things clearer?
-
Alexandros Frantzis (＠afrantzis)