dbus: Refactor to share DBus connections

We should avoid creating more than one session connection or more than
one system connection. We should also ideally avoid blocking the main
thread.

For now this still uses blocking in a couple places, but wrapping async
code (which is how `zbus::blocking` is implemented anyway).

This moves the `NameOwners` creation out of `A11yKeyboardMonitorState`,
so it can be shared with other things. We will likely want that for
https://github.com/pop-os/cosmic-comp/pull/465 to define a secured
protocol to pass an fd to the portal, and potentially any other
sensitive DBus protocols implemented by the compositor.

Cargo.lock

@@ -242,6 +242,12 @@ dependencies = [
  "pin-project-lite",
 ]
 
+[[package]]
+name = "async-once-cell"
+version = "0.5.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4288f83726785267c6f2ef073a3d83dc3f9b81464e9f99898240cced85fce35a"
+
 [[package]]
 name = "async-process"
 version = "2.5.0"
@@ -829,6 +835,7 @@ name = "cosmic-comp"
 version = "1.0.0"
 dependencies = [
  "anyhow",
+ "async-once-cell",
  "bitflags 2.11.0",
  "calloop 0.14.4",
  "cgmath",
@@ -867,7 +874,6 @@ dependencies = [
  "sanitize-filename",
  "serde",
  "serde_json",
- "smallvec",
  "smithay",
  "smithay-egui",
  "thiserror 2.0.18",

Cargo.toml

@@ -80,10 +80,10 @@ rand = "0.10"
 clap_lex = "1.0"
 parking_lot = "0.12.5"
 logind-zbus = { version = "5.3.2", optional = true }
-futures-executor = { version = "0.3.32", features = ["thread-pool"] }
+futures-executor = { version = "0.3.32" }
 futures-util = "0.3.32"
 cgmath = "0.18.0"
-smallvec = "1.15.1"
+async-once-cell = "0.5.4"
 
 [dependencies.id_tree]
 branch = "feature/copy_clone"

src/dbus/a11y_keyboard_monitor.rs

@@ -6,7 +6,7 @@ use smithay::{
 };
 use std::{
     collections::{HashMap, HashSet},
-    sync::{Arc, Mutex, OnceLock},
+    sync::{Arc, Mutex},
 };
 use tracing::debug;
 use xkbcommon::xkb::Keysym;
@@ -74,37 +74,34 @@ pub struct A11yKeyboardMonitorState {
     executor: calloop::futures::Scheduler<()>,
     clients: Arc<Mutex<Clients>>,
     active_virtual_mods: HashSet<Keysym>,
-    conn: Arc<OnceLock<zbus::Connection>>,
+    conn: zbus::Connection,
-    name_owners: Arc<OnceLock<NameOwners>>,
+    name_owners: NameOwners,
 }
 
 impl A11yKeyboardMonitorState {
-    pub fn new(executor: &calloop::futures::Scheduler<()>) -> Self {
+    pub async fn new(
+        conn: &zbus::Connection,
+        name_owners: &NameOwners,
+        executor: &calloop::futures::Scheduler<()>,
+    ) -> zbus::Result<Self> {
         let clients = Arc::new(Mutex::new(Clients::default()));
-        let clients_clone = clients.clone();
+
-        let conn_cell = Arc::new(OnceLock::new());
+        let keyboard_monitor = KeyboardMonitor {
-        let conn_cell_clone = conn_cell.clone();
+            clients: clients.clone(),
-        let name_owners_cell = Arc::new(OnceLock::new());
+            name_owners: name_owners.clone(),
-        let name_owners_cell_clone = name_owners_cell.clone();
+        };
-        let executor_clone = executor.clone();
+        conn.object_server()
-        let _ = executor.schedule(async move {
+            .at("/org/freedesktop/a11y/Manager", keyboard_monitor)
-            match serve(clients_clone, &executor_clone).await {
+            .await?;
-                Ok((conn, name_owners)) => {
+        conn.request_name("org.freedesktop.a11y.Manager").await?;
-                    conn_cell_clone.set(conn).unwrap();
+
-                    name_owners_cell_clone.set(name_owners).unwrap();
+        Ok(Self {
-                }
-                Err(err) => {
-                    tracing::error!("Failed to serve `org.freedesktop.a11y.Manager`: {err}");
-                }
-            }
-        });
-        Self {
             executor: executor.clone(),
             clients,
             active_virtual_mods: HashSet::new(),
-            conn: conn_cell,
+            conn: conn.clone(),
-            name_owners: name_owners_cell,
+            name_owners: name_owners.clone(),
-        }
+        })
     }
 
     pub fn has_virtual_mod(&self, keysym: Keysym) -> bool {
@@ -153,10 +150,6 @@ impl A11yKeyboardMonitorState {
     }
 
     pub fn key_event(&self, modifiers: &ModifiersState, keysym: &KeysymHandle, state: KeyState) {
-        let Some(conn) = self.conn.get() else {
-            return;
-        };
-
         let clients = self.clients.lock().unwrap();
         for (unique_name, client) in clients.0.iter() {
             if !client.watched && !self.has_key_grab(modifiers, keysym.modified_sym()) {
@@ -164,7 +157,7 @@ impl A11yKeyboardMonitorState {
             }
 
             let mut signal_context =
-                SignalEmitter::new(conn, "/org/freedesktop/a11y/Manager").unwrap();
+                SignalEmitter::new(&self.conn, "/org/freedesktop/a11y/Manager").unwrap();
             // Instead of sending signal to all clients, send only to authorized
             // clients with registed watches.
             signal_context = signal_context.set_destination(unique_name.clone().into());
@@ -194,13 +187,11 @@ impl A11yKeyboardMonitorState {
     pub fn refresh(&mut self) {
         // Remove clients and associated grabs when unique names are no longer
         // present on bus, or no longer hold approved name on bus.
-        if let Some(name_owners) = self.name_owners.get() {
+        self.clients
-            self.clients
+            .lock()
-                .lock()
+            .unwrap()
-                .unwrap()
+            .0
-                .0
+            .retain(|k, _| self.name_owners.check_owner_no_poll(k, ALLOWED_NAMES))
-                .retain(|k, _| name_owners.check_owner_no_poll(k, ALLOWED_NAMES))
-        }
     }
 }
 
@@ -310,20 +301,3 @@ impl KeyboardMonitor {
         keycode: u16,
     ) -> zbus::Result<()>;
 }
-
-async fn serve(
-    clients: Arc<Mutex<Clients>>,
-    executor: &calloop::futures::Scheduler<()>,
-) -> zbus::Result<(zbus::Connection, NameOwners)> {
-    let conn = zbus::Connection::session().await?;
-    let name_owners = NameOwners::new(&conn, executor).await?;
-    let keyboard_monitor = KeyboardMonitor {
-        clients,
-        name_owners: name_owners.clone(),
-    };
-    conn.object_server()
-        .at("/org/freedesktop/a11y/Manager", keyboard_monitor)
-        .await?;
-    conn.request_name("org.freedesktop.a11y.Manager").await?;
-    Ok((conn, name_owners))
-}

src/dbus/logind.rs

@@ -1,24 +1,34 @@
 use std::os::fd::OwnedFd;
 
 use anyhow::{Context, Result};
-use logind_zbus::manager::{InhibitType::HandleLidSwitch, ManagerProxyBlocking};
+use logind_zbus::manager::{InhibitType::HandleLidSwitch, ManagerProxy};
-use zbus::blocking::Connection;
 
-pub fn inhibit_lid() -> Result<OwnedFd> {
+use crate::state::Common;
-    let conn = Connection::system()?;
+
-    let proxy = ManagerProxyBlocking::new(&conn)?;
+pub fn inhibit_lid(common: &Common) -> Result<OwnedFd> {
-    let fd = proxy.inhibit(
+    let fd = futures_executor::block_on(async {
-        HandleLidSwitch,
+        let conn = common.dbus_state.system_conn().await?;
-        "cosmic-comp",
+        let manager = ManagerProxy::new(conn).await?;
-        "External output connected",
+        manager
-        "block",
+            .inhibit(
-    )?;
+                HandleLidSwitch,
+                "cosmic-comp",
+                "External output connected",
+                "block",
+            )
+            .await
+    })?;
 
     Ok(fd.into())
 }
 
-pub fn lid_closed() -> Result<bool> {
+pub fn lid_closed(common: &Common) -> Result<bool> {
-    let conn = Connection::system()?;
+    futures_executor::block_on(async {
-    let proxy = ManagerProxyBlocking::new(&conn)?;
+        let conn = common.dbus_state.system_conn().await?;
-    proxy.lid_closed().context("Failed to talk to logind")
+        let manager = ManagerProxy::new(conn).await?;
+        manager
+            .lid_closed()
+            .await
+            .context("Failed to talk to logind")
+    })
 }

src/dbus/mod.rs

@@ -5,21 +5,110 @@ use crate::{
 use anyhow::{Context, Result};
 use calloop::{InsertError, LoopHandle, stream::StreamSource};
 use cosmic_comp_config::output::comp::OutputState;
-use futures_executor::block_on;
+use std::{
-use std::collections::HashMap;
+    cell::{RefCell, RefMut},
+    collections::HashMap,
+    rc::Rc,
+};
 use tracing::{error, warn};
-use zbus::blocking::{Connection, fdo::DBusProxy};
 
 pub mod a11y_keyboard_monitor;
+use a11y_keyboard_monitor::A11yKeyboardMonitorState;
 #[cfg(feature = "systemd")]
 pub mod logind;
 mod name_owners;
 mod power;
 
-pub fn init(evlh: &LoopHandle<'static, State>) -> Result<()> {
+#[derive(Clone, Debug)]
-    match block_on(power::init()) {
+pub struct DBusState(Rc<DBusStateInner>);
+
+#[derive(Debug)]
+struct DBusStateInner {
+    evlh: LoopHandle<'static, State>,
+    executor: calloop::futures::Scheduler<()>,
+    session_conn: async_once_cell::OnceCell<zbus::Connection>,
+    system_conn: async_once_cell::OnceCell<zbus::Connection>,
+    a11y_keyboard_monitor: RefCell<Option<a11y_keyboard_monitor::A11yKeyboardMonitorState>>,
+}
+
+impl DBusState {
+    pub fn init(evlh: &LoopHandle<'static, State>) -> Self {
+        let (source, executor) = calloop::futures::executor().unwrap();
+        let state = Self(Rc::new(DBusStateInner {
+            evlh: evlh.clone(),
+            executor,
+            session_conn: async_once_cell::OnceCell::new(),
+            system_conn: async_once_cell::OnceCell::new(),
+            a11y_keyboard_monitor: RefCell::new(None),
+        }));
+        evlh.insert_source(source, |_, _, _| {}).unwrap();
+        let state_clone = state.clone();
+        state.spawn(async move {
+            if let Err(err) = init_session(&state_clone).await {
+                tracing::error!("Failed to initialize session DBus connection: {}", err);
+            }
+        });
+        let state_clone = state.clone();
+        state.spawn(async move {
+            if let Err(err) = init_system(&state_clone).await {
+                tracing::error!("Failed to initialize system DBus connection: {}", err);
+            }
+        });
+        state
+    }
+
+    pub fn a11y_keyboard_monitor(
+        &self,
+    ) -> Option<RefMut<'_, a11y_keyboard_monitor::A11yKeyboardMonitorState>> {
+        RefMut::filter_map(self.0.a11y_keyboard_monitor.borrow_mut(), |x| x.as_mut()).ok()
+    }
+
+    async fn session_conn(&self) -> zbus::Result<&zbus::Connection> {
+        self.0
+            .session_conn
+            .get_or_try_init(zbus::Connection::session())
+            .await
+    }
+
+    async fn system_conn(&self) -> zbus::Result<&zbus::Connection> {
+        self.0
+            .system_conn
+            .get_or_try_init(zbus::Connection::system())
+            .await
+    }
+
+    fn spawn(&self, fut: impl Future<Output = ()> + 'static) {
+        let _ = self.0.executor.schedule(fut);
+    }
+}
+
+async fn init_session(state: &DBusState) -> zbus::Result<()> {
+    let conn = state.session_conn().await?;
+    let name_owners = name_owners::NameOwners::new(conn, &state.0.executor).await?;
+    let a11y_keyboard_monitor_state =
+        A11yKeyboardMonitorState::new(conn, &name_owners, &state.0.executor).await?;
+    *state.0.a11y_keyboard_monitor.borrow_mut() = Some(a11y_keyboard_monitor_state);
+    Ok(())
+}
+
+async fn init_system(state: &DBusState) -> zbus::Result<()> {
+    let conn = state.system_conn().await?.clone();
+    let evlh = state.0.evlh.clone();
+    state.spawn(async move {
+        if let Err(err) = power_hot_plug_task(conn, evlh).await {
+            tracing::warn!(?err, "Failed to initialize dbus handlers");
+        }
+    });
+    Ok(())
+}
+
+async fn power_hot_plug_task(
+    conn: zbus::Connection,
+    evlh: LoopHandle<'static, State>,
+) -> Result<()> {
+    match power::init(&conn).await {
         Ok(power_daemon) => {
-            if let Ok(stream) = block_on(power_daemon.receive_hot_plug_detect()) {
+            if let Ok(stream) = power_daemon.receive_hot_plug_detect().await {
                 let source = StreamSource::new(stream).unwrap();
                 evlh.insert_source(source, |_, _, state| {
                     let nodes = match &mut state.backend {
@@ -63,20 +152,22 @@ pub fn init(evlh: &LoopHandle<'static, State>) -> Result<()> {
 /// Updated the D-Bus activation environment with `WAYLAND_DISPLAY` and
 /// `DISPLAY` variables.
 pub fn ready(common: &Common) -> Result<()> {
-    let conn = Connection::session()?;
+    futures_executor::block_on(async {
-    let proxy = DBusProxy::new(&conn)?;
+        let conn = common.dbus_state.session_conn().await?;
-
+        let dbus = zbus::fdo::DBusProxy::new(conn).await?;
-    proxy.update_activation_environment(HashMap::from([
-        ("WAYLAND_DISPLAY", common.socket.to_str().unwrap()),
-        (
-            "DISPLAY",
-            &common
-                .xwayland_state
-                .as_ref()
-                .map(|s| format!(":{}", s.display))
-                .unwrap_or_default(),
-        ),
-    ]))?;
 
+        dbus.update_activation_environment(HashMap::from([
+            ("WAYLAND_DISPLAY", common.socket.to_str().unwrap()),
+            (
+                "DISPLAY",
+                &common
+                    .xwayland_state
+                    .as_ref()
+                    .map(|s| format!(":{}", s.display))
+                    .unwrap_or_default(),
+            ),
+        ]))
+        .await
+    })?;
     Ok(())
 }

src/dbus/power.rs

@@ -79,9 +79,8 @@ pub trait PowerDaemon {
     fn power_profile_switch(&self, profile: &str) -> zbus::Result<()>;
 }
 
-pub async fn init() -> anyhow::Result<PowerDaemonProxy<'static>> {
+pub async fn init(conn: &Connection) -> anyhow::Result<PowerDaemonProxy<'static>> {
-    let conn = Connection::system().await?;
+    let proxy = PowerDaemonProxy::new(conn).await?;
-    let proxy = PowerDaemonProxy::new(&conn).await?;
     proxy.0.introspect().await?;
     Ok(proxy)
 }

src/input/mod.rs

@@ -1668,9 +1668,9 @@ impl State {
                 .unwrap_or(false)
         });
 
-        self.common
+        if let Some(a11y_keyboard_monitor) = self.common.dbus_state.a11y_keyboard_monitor() {
-            .a11y_keyboard_monitor_state
+            a11y_keyboard_monitor.key_event(modifiers, &handle, event.state());
-            .key_event(modifiers, &handle, event.state());
+        }
 
         // Leave move overview mode, if any modifier was released
         if let Some(Trigger::KeyboardMove(action_modifiers)) =
@@ -1830,61 +1830,54 @@ impl State {
             )));
         }
 
-        if event.state() == KeyState::Released {
+        if let Some(mut a11y_keyboard_monitor) = self.common.dbus_state.a11y_keyboard_monitor() {
-            let removed = self
+            if event.state() == KeyState::Released {
-                .common
+                let removed =
-                .a11y_keyboard_monitor_state
+                    a11y_keyboard_monitor.remove_active_virtual_mod(handle.modified_sym());
-                .remove_active_virtual_mod(handle.modified_sym());
+                // If `Caps_Lock` is a virtual modifier, and is in locked state, clear it
-            // If `Caps_Lock` is a virtual modifier, and is in locked state, clear it
+                if removed
-            if removed
+                    && handle.modified_sym() == Keysym::Caps_Lock
-                && handle.modified_sym() == Keysym::Caps_Lock
+                    && (modifiers.serialized.locked & 2) != 0
-                && (modifiers.serialized.locked & 2) != 0
+                {
+                    let seat = seat.clone();
+                    let key_code = event.key_code();
+                    self.common.event_loop_handle.insert_idle(move |state| {
+                        if let Some(keyboard) = seat.get_keyboard() {
+                            let serial = SERIAL_COUNTER.next_serial();
+                            let time = state.common.clock.now().as_millis();
+                            keyboard.input(
+                                state,
+                                key_code,
+                                KeyState::Pressed,
+                                serial,
+                                time,
+                                |_, _, _| FilterResult::<()>::Forward,
+                            );
+                            let serial = SERIAL_COUNTER.next_serial();
+                            keyboard.input(
+                                state,
+                                key_code,
+                                KeyState::Released,
+                                serial,
+                                time,
+                                |_, _, _| FilterResult::<()>::Forward,
+                            );
+                        }
+                    });
+                }
+            } else if event.state() == KeyState::Pressed
+                && a11y_keyboard_monitor.has_virtual_mod(handle.modified_sym())
             {
-                let seat = seat.clone();
+                a11y_keyboard_monitor.add_active_virtual_mod(handle.modified_sym());
-                let key_code = event.key_code();
+
-                self.common.event_loop_handle.insert_idle(move |state| {
+                tracing::debug!(
-                    if let Some(keyboard) = seat.get_keyboard() {
+                    "active virtual mods: {:?}",
-                        let serial = SERIAL_COUNTER.next_serial();
+                    a11y_keyboard_monitor.active_virtual_mods()
-                        let time = state.common.clock.now().as_millis();
+                );
-                        keyboard.input(
+                seat.supressed_keys().add(&handle, None);
-                            state,
+
-                            key_code,
+                return FilterResult::Intercept(None);
-                            KeyState::Pressed,
-                            serial,
-                            time,
-                            |_, _, _| FilterResult::<()>::Forward,
-                        );
-                        let serial = SERIAL_COUNTER.next_serial();
-                        keyboard.input(
-                            state,
-                            key_code,
-                            KeyState::Released,
-                            serial,
-                            time,
-                            |_, _, _| FilterResult::<()>::Forward,
-                        );
-                    }
-                });
             }
-        } else if event.state() == KeyState::Pressed
-            && self
-                .common
-                .a11y_keyboard_monitor_state
-                .has_virtual_mod(handle.modified_sym())
-        {
-            self.common
-                .a11y_keyboard_monitor_state
-                .add_active_virtual_mod(handle.modified_sym());
-
-            tracing::debug!(
-                "active virtual mods: {:?}",
-                self.common
-                    .a11y_keyboard_monitor_state
-                    .active_virtual_mods()
-            );
-            seat.supressed_keys().add(&handle, None);
-
-            return FilterResult::Intercept(None);
         }
 
         // Skip released events for initially surpressed keys
@@ -1911,12 +1904,10 @@ impl State {
             return FilterResult::Intercept(None);
         }
 
-        if event.state() == KeyState::Pressed
+        if let Some(a11y_keyboard_monitor) = self.common.dbus_state.a11y_keyboard_monitor()
-            && (self.common.a11y_keyboard_monitor_state.has_keyboard_grab()
+            && event.state() == KeyState::Pressed
-                || self
+            && (a11y_keyboard_monitor.has_keyboard_grab()
-                    .common
+                || a11y_keyboard_monitor.has_key_grab(modifiers, handle.modified_sym()))
-                    .a11y_keyboard_monitor_state
-                    .has_key_grab(modifiers, handle.modified_sym()))
         {
             let modifiers_queue = seat.modifiers_shortcut_queue();
             modifiers_queue.clear();

src/shell/mod.rs

@@ -1577,7 +1577,9 @@ impl Common {
         self.popups.cleanup();
         self.toplevel_info_state.refresh(&self.workspace_state);
         self.refresh_idle_inhibit();
-        self.a11y_keyboard_monitor_state.refresh();
+        if let Some(mut a11y_keyboard_monitor) = self.dbus_state.a11y_keyboard_monitor() {
+            a11y_keyboard_monitor.refresh();
+        }
         self.image_copy_capture_state.cleanup();
     }
 

src/state.rs

@@ -8,7 +8,7 @@ use crate::{
         x11::X11State,
     },
     config::{CompOutputConfig, Config, ScreenFilter},
-    dbus::a11y_keyboard_monitor::A11yKeyboardMonitorState,
+    dbus::DBusState,
     input::{PointerFocusState, gestures::GestureState},
     shell::{CosmicSurface, SeatExt, Shell, grabs::SeatMoveGrabState},
     utils::prelude::OutputExt,
@@ -233,7 +233,6 @@ pub struct Common {
     pub display_handle: DisplayHandle,
     pub event_loop_handle: LoopHandle<'static, State>,
     pub event_loop_signal: LoopSignal,
-    pub async_executor: calloop::futures::Scheduler<()>,
 
     pub popups: PopupManager,
     pub shell: Arc<parking_lot::RwLock<Shell>>,
@@ -278,7 +277,7 @@ pub struct Common {
     pub xdg_decoration_state: XdgDecorationState,
     pub overlap_notify_state: OverlapNotifyState,
     pub a11y_state: A11yState,
-    pub a11y_keyboard_monitor_state: A11yKeyboardMonitorState,
+    pub dbus_state: DBusState,
 
     // shell-related wayland state
     pub xdg_shell_state: XdgShellState,
@@ -726,16 +725,9 @@ impl State {
         );
         let workspace_state = WorkspaceState::new(dh, client_not_sandboxed);
 
-        let (source, async_executor) = calloop::futures::executor().unwrap();
-        handle.insert_source(source, |_, _, _| {}).unwrap();
-
-        if let Err(err) = crate::dbus::init(&handle) {
-            tracing::warn!(?err, "Failed to initialize dbus handlers");
-        }
-
         let a11y_state = A11yState::new::<State, _>(dh, client_not_sandboxed);
 
-        let a11y_keyboard_monitor_state = A11yKeyboardMonitorState::new(&async_executor);
+        let dbus_state = DBusState::init(&handle);
 
         State {
             common: Common {
@@ -744,7 +736,6 @@ impl State {
                 display_handle: dh.clone(),
                 event_loop_handle: handle,
                 event_loop_signal: signal,
-                async_executor,
 
                 popups: PopupManager::default(),
                 shell,
@@ -794,11 +785,11 @@ impl State {
                 xdg_foreign_state,
                 workspace_state,
                 a11y_state,
-                a11y_keyboard_monitor_state,
                 xwayland_scale: None,
                 xwayland_state: None,
                 xwayland_shell_state,
                 pointer_focus_state: None,
+                dbus_state,
 
                 #[cfg(feature = "systemd")]
                 inhibit_lid_fd: None,
@@ -841,7 +832,7 @@ impl State {
 
             if should_handle_lid {
                 if self.common.inhibit_lid_fd.is_none() {
-                    match crate::dbus::logind::inhibit_lid() {
+                    match crate::dbus::logind::inhibit_lid(&self.common) {
                         Ok(fd) => {
                             debug!("Inhibiting lid switch");
                             self.common.inhibit_lid_fd = Some(fd);
@@ -852,7 +843,8 @@ impl State {
                                 .iter()
                                 .find(|o| o.is_internal())
                                 .cloned();
-                            let closed = crate::dbus::logind::lid_closed().unwrap_or(false);
+                            let closed =
+                                crate::dbus::logind::lid_closed(&self.common).unwrap_or(false);
 
                             if closed {
                                 backend.disable_internal_output(