From c1bf410466bff62c58f2115b4a5f2e4861592d6b Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 31 Jul 2024 14:49:10 -0700 Subject: [PATCH] toplevel-info: Fix race between binding `wl_ouput` and `output_enter` This fixes an issue with `cosmic-panel` where, when a workspace is moved back to an output after a monitor is disconnected and reconnected, the panel doesn't hide because `cosmic-panel` thinks no toplevel is open on that monitor. After some testing, it seems `output_enter` isn't being sent here. In particular, the `output_leave` call happens before the client binds the `wl_output`, so there is no `wl_output` to send in an event yet. This is addressed by keeping track of a set of `wl_output`s that we have sent the event to. So if an output is bound, `refresh` can add it to this list and send the event. This is not needed for workspaces (though it could be done similarly) since the handle objects are created by server events. So no race should occur as long as the workspaces global is bound before the toplevel info one. --- src/wayland/protocols/toplevel_info.rs | 48 ++++++++++++++------------ 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/src/wayland/protocols/toplevel_info.rs b/src/wayland/protocols/toplevel_info.rs index ce094ca1..7e9fd36e 100644 --- a/src/wayland/protocols/toplevel_info.rs +++ b/src/wayland/protocols/toplevel_info.rs @@ -1,12 +1,12 @@ // SPDX-License-Identifier: GPL-3.0-only -use std::sync::Mutex; +use std::{collections::HashSet, sync::Mutex}; use smithay::{ output::Output, reexports::wayland_server::{ backend::{ClientId, GlobalId}, - protocol::wl_surface::WlSurface, + protocol::{wl_output::WlOutput, wl_surface::WlSurface}, Client, DataInit, Dispatch, DisplayHandle, GlobalDispatch, New, Resource, Weak, }, utils::{user_data::UserDataMap, IsAlive, Logical, Rectangle}, @@ -59,6 +59,7 @@ pub(super) type ToplevelState = Mutex; pub struct ToplevelHandleStateInner { outputs: Vec, + wl_outputs: HashSet, workspaces: Vec, title: String, app_id: String, @@ -71,6 +72,7 @@ impl ToplevelHandleStateInner { fn from_window(window: &W) -> ToplevelHandleState { ToplevelHandleState::new(ToplevelHandleStateInner { outputs: Vec::new(), + wl_outputs: HashSet::new(), workspaces: Vec::new(), title: String::new(), app_id: String::new(), @@ -399,27 +401,29 @@ fn send_toplevel_to_client( } if let Ok(client) = dh.get_client(instance.id()) { - for new_output in state - .outputs - .iter() - .filter(|o| !handle_state.outputs.contains(o)) - { - for wl_output in new_output.client_outputs(&client) { - instance.output_enter(&wl_output); - } - changed = true; - } - for old_output in handle_state - .outputs - .iter() - .filter(|o| !state.outputs.contains(o)) - { - for wl_output in old_output.client_outputs(&client) { - instance.output_leave(&wl_output); - } - changed = true; - } handle_state.outputs = state.outputs.clone(); + + let handle_state = &mut *handle_state; + for output in &handle_state.outputs { + for wl_output in output.client_outputs(&client) { + if handle_state.wl_outputs.insert(wl_output.clone()) { + instance.output_enter(&wl_output); + changed = true; + } + } + } + handle_state.wl_outputs.retain(|wl_output| { + let retain = wl_output.is_alive() + && handle_state + .outputs + .iter() + .any(|output| output.owns(wl_output)); + if !retain { + instance.output_leave(&wl_output); + changed = true; + } + retain + }); } for new_workspace in state