Skip to content

Commit

Permalink
fix(clipboard): possible deadlock when copying (#863)
Browse files Browse the repository at this point in the history 
* Change clipboard calloop event loop with blocking read to tokio async readers.

Co-authored-by: Ridan Vandenbergh <[email protected]>

* cargo fmt

---------

Co-authored-by: Ridan Vandenbergh <[email protected]>
  • Loading branch information
@ToxicMushroom @zeroeightysix
ToxicMushroom and zeroeightysix authored Feb 6, 2025
1 parent 68ccc7a commit 75375aa
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 82 deletions.
2 changes: 1 addition & 1 deletion .idea/runConfigurations/Run__Live_Config_.xml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 1 addition & 8 deletions src/clients/wayland/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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>,
Expand All @@ -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")]
Expand Down Expand Up @@ -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"))]
Expand All @@ -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),
};

Expand Down
88 changes: 24 additions & 64 deletions src/clients/wayland/wlr_data_control/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,31 @@ 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};
use wayland_protocols_wlr::data_control::v1::client::zwlr_data_control_source_v1::ZwlrDataControlSourceV1;

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.
///
Expand Down Expand Up @@ -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)
}
};
Expand Down Expand Up @@ -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();
Expand All @@ -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:?}"),
}
});
}
}
}
Expand Down
15 changes: 6 additions & 9 deletions src/clients/wayland/wlr_data_control/offer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)?)
}

0 comments on commit 75375aa

Please sign in to comment.