When removing output global, use disable_global, remove with timer

This should fix an issue where output hotplug can sometimes cause
clients (including XWayland) to crash with a protocol error trying to
bind the output.

Using a timer doesn't seem ideal, but seems to be the correct way to do
this at present. Wlroots `wlr_global_destroy_safe` is basically the same
as this.

Adding a `LoopHandle` argument to `OutputConfigurationState::new` seems
awkward, but maybe better than a handler method for removing globals.
(`IdleNotifierState::new` also takes a `LoopHandle`). Perhaps Smithay
could provide some kind of helper for this.

Author: ids1024

src/state.rs

@@ -499,7 +499,8 @@ impl State {
         let fractional_scale_state = FractionalScaleManagerState::new::<State>(dh);
         let keyboard_shortcuts_inhibit_state = KeyboardShortcutsInhibitState::new::<Self>(dh);
         let output_state = OutputManagerState::new_with_xdg_output::<Self>(dh);
-        let output_configuration_state = OutputConfigurationState::new(dh, client_is_privileged);
+        let output_configuration_state =
+            OutputConfigurationState::new(dh, handle.clone(), client_is_privileged);
         let output_power_state = OutputPowerState::new::<Self, _>(dh, client_is_privileged);
         let overlap_notify_state =
             OverlapNotifyState::new::<Self, _>(dh, client_has_no_security_context);

src/wayland/protocols/output_configuration/mod.rs

@@ -1,5 +1,9 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
+use calloop::{
+    timer::{TimeoutAction, Timer},
+    LoopHandle,
+};
 use cosmic_protocols::output_management::v1::server::{
     zcosmic_output_configuration_head_v1::ZcosmicOutputConfigurationHeadV1,
     zcosmic_output_configuration_v1::ZcosmicOutputConfigurationV1,
@@ -25,7 +29,7 @@ use smithay::{
     utils::{Logical, Physical, Point, Size, Transform},
     wayland::output::WlOutputData,
 };
-use std::{convert::TryFrom, sync::Mutex};
+use std::{convert::TryFrom, sync::Mutex, time::Duration};
 
 mod handlers;
 
@@ -45,6 +49,7 @@ pub struct OutputConfigurationState<D> {
     global: GlobalId,
     extension_global: GlobalId,
     dh: DisplayHandle,
+    event_loop_handle: LoopHandle<'static, D>,
     _dispatch: std::marker::PhantomData<D>,
 }
 
@@ -168,7 +173,11 @@ where
         + OutputConfigurationHandler
         + 'static,
 {
-    pub fn new<F>(dh: &DisplayHandle, client_filter: F) -> OutputConfigurationState<D>
+    pub fn new<F>(
+        dh: &DisplayHandle,
+        event_loop_handle: LoopHandle<'static, D>,
+        client_filter: F,
+    ) -> OutputConfigurationState<D>
     where
         F: for<'a> Fn(&'a Client) -> bool + Clone + Send + Sync + 'static,
     {
@@ -194,6 +203,7 @@ where
             global,
             extension_global,
             dh: dh.clone(),
+            event_loop_handle: event_loop_handle.clone(),
             _dispatch: std::marker::PhantomData,
         }
     }
@@ -240,7 +250,7 @@ where
                     let mut inner = inner.lock().unwrap();
                     inner.enabled = false;
                     if let Some(global) = inner.global.take() {
-                        self.dh.remove_global::<D>(global);
+                        remove_global_with_timer(&self.dh, &self.event_loop_handle, global);
                     }
                 }
             }
@@ -292,7 +302,11 @@ where
                     inner.global = Some(output.create_global::<D>(&self.dh));
                 }
                 if !inner.enabled && inner.global.is_some() {
-                    self.dh.remove_global::<D>(inner.global.take().unwrap());
+                    remove_global_with_timer(
+                        &self.dh,
+                        &self.event_loop_handle,
+                        inner.global.take().unwrap(),
+                    );
                 }
             }
             for manager in self.instances.iter_mut() {
@@ -493,6 +507,26 @@ where
     }
 }
 
+fn remove_global_with_timer<D: 'static>(
+    dh: &DisplayHandle,
+    event_loop_handle: &LoopHandle<D>,
+    id: GlobalId,
+) {
+    dh.disable_global::<D>(id.clone());
+    let source = Timer::from_duration(Duration::from_secs(5));
+    let dh = dh.clone();
+    let res = event_loop_handle.insert_source(source, move |_, _, _state| {
+        dh.remove_global::<D>(id.clone());
+        TimeoutAction::Drop
+    });
+    if let Err(err) = res {
+        tracing::error!(
+            "failed to insert timer source to destroy output global: {}",
+            err
+        );
+    }
+}
+
 macro_rules! delegate_output_configuration {
     ($(@<$( $lt:tt $( : $clt:tt $(+ $dlt:tt )* )? ),+>)? $ty: ty) => {
         smithay::reexports::wayland_server::delegate_global_dispatch!($(@< $( $lt $( : $clt $(+ $dlt )* )? ),+ >)? $ty: [