From 75375aa341ec154345a7f41d210c952d4ffe2002 Mon Sep 17 00:00:00 2001
From: Merlijn <32853531+ToxicMushroom@users.noreply.github.com>
Date: Thu, 6 Feb 2025 17:29:33 +0100
Subject: [PATCH] fix(clipboard): possible deadlock when copying (#863)

* Change clipboard calloop event loop with blocking read to tokio async readers.

Co-authored-by: Ridan Vandenbergh <ridanvandenbergh@gmail.com>

* cargo fmt

---------

Co-authored-by: Ridan Vandenbergh <ridanvandenbergh@gmail.com>
---
 .idea/runConfigurations/Run__Live_Config_.xml |  2 +-
 src/clients/wayland/mod.rs                    |  9 +-
 src/clients/wayland/wlr_data_control/mod.rs   | 88 +++++--------------
 src/clients/wayland/wlr_data_control/offer.rs | 15 ++--
 4 files changed, 32 insertions(+), 82 deletions(-)

diff --git a/.idea/runConfigurations/Run__Live_Config_.xml b/.idea/runConfigurations/Run__Live_Config_.xml
index f5f4c0e9..c54a85c3 100644
--- a/.idea/runConfigurations/Run__Live_Config_.xml
+++ b/.idea/runConfigurations/Run__Live_Config_.xml
@@ -11,7 +11,7 @@
     <option name="backtrace" value="SHORT" />
     <envs>
       <env name="PATH" value="/usr/local/bin:/usr/bin:$USER_HOME$/.local/share/npm/bin" />
-      <env name="RUST_LOG" value="debug" />
+      <env name="IRONBAR_LOG" value="debug" />
     </envs>
     <option name="isRedirectInput" value="false" />
     <option name="redirectInputPath" value="" />
diff --git a/src/clients/wayland/mod.rs b/src/clients/wayland/mod.rs
index dc201f51..ac09cad3 100644
--- a/src/clients/wayland/mod.rs
+++ b/src/clients/wayland/mod.rs
@@ -12,7 +12,7 @@ use cfg_if::cfg_if;
 use color_eyre::Report;
 use smithay_client_toolkit::output::OutputState;
 use smithay_client_toolkit::reexports::calloop::channel as calloop_channel;
-use smithay_client_toolkit::reexports::calloop::{EventLoop, LoopHandle};
+use smithay_client_toolkit::reexports::calloop::EventLoop;
 use smithay_client_toolkit::reexports::calloop_wayland_source::WaylandSource;
 use smithay_client_toolkit::registry::{ProvidesRegistryState, RegistryState};
 use smithay_client_toolkit::seat::SeatState;
@@ -43,7 +43,6 @@ cfg_if! {
         use self::wlr_data_control::device::DataControlDevice;
         use self::wlr_data_control::manager::DataControlDeviceManagerState;
         use self::wlr_data_control::source::CopyPasteSource;
-        use self::wlr_data_control::SelectionOfferItem;
         use wayland_client::protocol::wl_seat::WlSeat;
 
         pub use wlr_data_control::{ClipboardItem, ClipboardValue};
@@ -195,7 +194,6 @@ pub struct Environment {
     seat_state: SeatState,
 
     queue_handle: QueueHandle<Self>,
-    loop_handle: LoopHandle<'static, Self>,
 
     event_tx: mpsc::Sender<Event>,
     response_tx: std::sync::mpsc::Sender<Response>,
@@ -212,8 +210,6 @@ pub struct Environment {
     data_control_devices: Vec<DataControlDeviceEntry>,
     #[cfg(feature = "clipboard")]
     copy_paste_sources: Vec<CopyPasteSource>,
-    #[cfg(feature = "clipboard")]
-    selection_offers: Vec<SelectionOfferItem>,
 
     // local state
     #[cfg(feature = "clipboard")]
@@ -281,7 +277,6 @@ impl Environment {
             #[cfg(feature = "clipboard")]
             data_control_device_manager_state,
             queue_handle: qh,
-            loop_handle: loop_handle.clone(),
             event_tx,
             response_tx,
             #[cfg(any(feature = "focused", feature = "launcher"))]
@@ -292,8 +287,6 @@ impl Environment {
             #[cfg(feature = "clipboard")]
             copy_paste_sources: vec![],
             #[cfg(feature = "clipboard")]
-            selection_offers: vec![],
-            #[cfg(feature = "clipboard")]
             clipboard: arc_mut!(None),
         };
 
diff --git a/src/clients/wayland/wlr_data_control/mod.rs b/src/clients/wayland/wlr_data_control/mod.rs
index dbe5f8e9..18911557 100644
--- a/src/clients/wayland/wlr_data_control/mod.rs
+++ b/src/clients/wayland/wlr_data_control/mod.rs
@@ -4,23 +4,24 @@ pub mod offer;
 pub mod source;
 
 use self::device::{DataControlDeviceDataExt, DataControlDeviceHandler};
-use self::offer::{DataControlDeviceOffer, DataControlOfferHandler, SelectionOffer};
+use self::offer::{DataControlDeviceOffer, DataControlOfferHandler};
 use self::source::DataControlSourceHandler;
 use super::{Client, Environment, Event, Request, Response};
-use crate::{lock, try_send, Ironbar};
+use crate::{lock, spawn, try_send, Ironbar};
 use device::DataControlDevice;
 use glib::Bytes;
 use nix::fcntl::{fcntl, F_GETPIPE_SZ, F_SETPIPE_SZ};
 use nix::sys::epoll::{Epoll, EpollCreateFlags, EpollEvent, EpollFlags, EpollTimeout};
 use smithay_client_toolkit::data_device_manager::WritePipe;
-use smithay_client_toolkit::reexports::calloop::{PostAction, RegistrationToken};
 use std::cmp::min;
 use std::fmt::{Debug, Formatter};
 use std::fs::File;
-use std::io::{ErrorKind, Read, Write};
-use std::os::fd::{AsRawFd, OwnedFd, RawFd};
+use std::io::{ErrorKind, Write};
+use std::os::fd::RawFd;
+use std::os::fd::{AsRawFd, OwnedFd};
 use std::sync::Arc;
 use std::{fs, io};
+use tokio::io::AsyncReadExt;
 use tokio::sync::broadcast;
 use tracing::{debug, error, trace};
 use wayland_client::{Connection, QueueHandle};
@@ -28,12 +29,6 @@ use wayland_protocols_wlr::data_control::v1::client::zwlr_data_control_source_v1
 
 const INTERNAL_MIME_TYPE: &str = "x-ironbar-internal";
 
-#[derive(Debug)]
-pub struct SelectionOfferItem {
-    offer: SelectionOffer,
-    token: Option<RegistrationToken>,
-}
-
 /// Represents a value which can be read/written
 /// to/from the system clipboard and surrounding metadata.
 ///
@@ -168,22 +163,20 @@ impl Environment {
     }
 
     /// Reads an offer file handle into a new `ClipboardItem`.
-    fn read_file(mime_type: &MimeType, file: &mut File) -> io::Result<ClipboardItem> {
+    async fn read_file(
+        mime_type: &MimeType,
+        file: &mut tokio::net::unix::pipe::Receiver,
+    ) -> io::Result<ClipboardItem> {
+        let mut buf = vec![];
+        file.read_to_end(&mut buf).await?;
+
         let value = match mime_type.category {
             MimeTypeCategory::Text => {
-                let mut txt = String::new();
-                file.read_to_string(&mut txt)?;
-
+                let txt = String::from_utf8_lossy(&buf).to_string();
                 ClipboardValue::Text(txt)
             }
             MimeTypeCategory::Image => {
-                let mut bytes = vec![];
-                file.read_to_end(&mut bytes)?;
-
-                debug!("Read bytes: {}", bytes.len());
-
-                let bytes = Bytes::from(&bytes);
-
+                let bytes = Bytes::from(&buf);
                 ClipboardValue::Image(bytes)
             }
         };
@@ -214,14 +207,6 @@ impl DataControlDeviceHandler for Environment {
         }
 
         if let Some(offer) = data_device.selection_offer() {
-            self.selection_offers
-                .push(SelectionOfferItem { offer, token: None });
-
-            let cur_offer = self
-                .selection_offers
-                .last_mut()
-                .expect("Failed to get current offer");
-
             // clear prev
             let Some(mime_type) = MimeType::parse_multiple(&mime_types) else {
                 lock!(self.clipboard).take();
@@ -238,44 +223,19 @@ impl DataControlDeviceHandler for Environment {
             };
 
             debug!("Receiving mime type: {}", mime_type.value);
-
-            if let Ok(read_pipe) = cur_offer.offer.receive(mime_type.value.clone()) {
-                let offer_clone = cur_offer.offer.clone();
-
+            if let Ok(mut read_pipe) = offer.receive(mime_type.value.clone()) {
                 let tx = self.event_tx.clone();
                 let clipboard = self.clipboard.clone();
 
-                let token =
-                    self.loop_handle
-                        .insert_source(read_pipe, move |(), file, state| unsafe {
-                            let item = state
-                                .selection_offers
-                                .iter()
-                                .position(|o| o.offer == offer_clone)
-                                .map(|p| state.selection_offers.remove(p))
-                                .expect("Failed to find selection offer item");
-
-                            match Self::read_file(&mime_type, file.get_mut()) {
-                                Ok(item) => {
-                                    lock!(clipboard).replace(item.clone());
-                                    try_send!(tx, Event::Clipboard(item));
-                                }
-                                Err(err) => error!("{err:?}"),
-                            }
-
-                            state
-                                .loop_handle
-                                .remove(item.token.expect("Missing item token"));
-
-                            PostAction::Remove
-                        });
-
-                match token {
-                    Ok(token) => {
-                        cur_offer.token.replace(token);
+                spawn(async move {
+                    match Self::read_file(&mime_type, &mut read_pipe).await {
+                        Ok(item) => {
+                            lock!(clipboard).replace(item.clone());
+                            try_send!(tx, Event::Clipboard(item));
+                        }
+                        Err(err) => error!("{err:?}"),
                     }
-                    Err(err) => error!("Failed to insert read pipe event: {err:?}"),
-                }
+                });
             }
         }
     }
diff --git a/src/clients/wayland/wlr_data_control/offer.rs b/src/clients/wayland/wlr_data_control/offer.rs
index f140db14..4b58bd15 100644
--- a/src/clients/wayland/wlr_data_control/offer.rs
+++ b/src/clients/wayland/wlr_data_control/offer.rs
@@ -3,10 +3,10 @@ use crate::lock;
 use nix::fcntl::OFlag;
 use nix::unistd::pipe2;
 use smithay_client_toolkit::data_device_manager::data_offer::DataOfferError;
-use smithay_client_toolkit::data_device_manager::ReadPipe;
 use std::ops::DerefMut;
 use std::os::fd::AsFd;
 use std::sync::{Arc, Mutex};
+use tokio::net::unix::pipe::Receiver;
 use tracing::trace;
 use wayland_client::{Connection, Dispatch, Proxy, QueueHandle};
 use wayland_protocols_wlr::data_control::v1::client::zwlr_data_control_offer_v1::{
@@ -36,8 +36,8 @@ impl PartialEq for SelectionOffer {
 }
 
 impl SelectionOffer {
-    pub fn receive(&self, mime_type: String) -> Result<ReadPipe, DataOfferError> {
-        unsafe { receive(&self.data_offer, mime_type) }.map_err(DataOfferError::Io)
+    pub fn receive(&self, mime_type: String) -> Result<Receiver, DataOfferError> {
+        receive(&self.data_offer, mime_type).map_err(DataOfferError::Io)
     }
 }
 
@@ -169,14 +169,11 @@ where
 ///
 /// Fails if too many file descriptors were already open and a pipe
 /// could not be created.
-pub unsafe fn receive(
-    offer: &ZwlrDataControlOfferV1,
-    mime_type: String,
-) -> std::io::Result<ReadPipe> {
+pub fn receive(offer: &ZwlrDataControlOfferV1, mime_type: String) -> std::io::Result<Receiver> {
     // create a pipe
-    let (readfd, writefd) = pipe2(OFlag::O_CLOEXEC)?;
+    let (readfd, writefd) = pipe2(OFlag::O_CLOEXEC | OFlag::O_NONBLOCK)?;
 
     offer.receive(mime_type, writefd.as_fd());
 
-    Ok(ReadPipe::from(readfd))
+    Ok(Receiver::from_owned_fd(readfd)?)
 }