Skip to content

Unsound synchronization on Linux #1173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sftse wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Unsound synchronization on Linux #1173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0463f18
refactor: remove unneeded unsafe impls
sftse Dec 18, 2025
29967cb
fix: ensure proper synchronization across threads setting window prop…
sftse Jan 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions src/platform_impl/linux/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use std::{
cell::RefCell,
collections::VecDeque,
rc::Rc,
sync::atomic::{AtomicBool, AtomicI32, Ordering},
sync::{
atomic::{AtomicBool, AtomicI32, Ordering},
RwLock,
},
};

use gtk::{
Expand Down Expand Up @@ -63,11 +66,11 @@ pub struct Window {
maximized: Rc<AtomicBool>,
is_always_on_top: Rc<AtomicBool>,
minimized: Rc<AtomicBool>,
fullscreen: RefCell<Option<Fullscreen>>,
inner_size_constraints: RefCell<WindowSizeConstraints>,
fullscreen: RwLock<Option<Fullscreen>>,
inner_size_constraints: RwLock<WindowSizeConstraints>,
/// Draw event Sender
draw_tx: crossbeam_channel::Sender<WindowId>,
preferred_theme: RefCell<Option<Theme>>,
preferred_theme: RwLock<Option<Theme>>,
css_provider: CssProvider,
}

Expand Down Expand Up @@ -288,9 +291,9 @@ impl Window {
maximized,
minimized,
is_always_on_top,
fullscreen: RefCell::new(attributes.fullscreen),
inner_size_constraints: RefCell::new(attributes.inner_size_constraints),
preferred_theme: RefCell::new(preferred_theme),
fullscreen: RwLock::new(attributes.fullscreen),
Copy link
Contributor

@Legend-Master Legend-Master Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the chances for read/write are not different by too much, maybe we should use Mutex here?

Copy link
Contributor Author

@sftse sftse Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used a RwLock because that is the most direct port of a RefCell to the multithreaded case. A Mutex can't have multiple readers, RwLock and RefCell can.

But scopes here make it clear there can't be deadlocks regardless of Mutex or RwLock, so we can move to Mutex.

inner_size_constraints: RwLock::new(attributes.inner_size_constraints),
preferred_theme: RwLock::new(preferred_theme),
css_provider: CssProvider::new(),
};

Expand Down Expand Up @@ -431,9 +434,9 @@ impl Window {
maximized,
minimized,
is_always_on_top,
fullscreen: RefCell::new(None),
inner_size_constraints: RefCell::new(WindowSizeConstraints::default()),
preferred_theme: RefCell::new(None),
fullscreen: RwLock::new(None),
inner_size_constraints: RwLock::new(WindowSizeConstraints::default()),
preferred_theme: RwLock::new(None),
css_provider: CssProvider::new(),
};

Expand Down Expand Up @@ -535,22 +538,22 @@ impl Window {

pub fn set_min_inner_size(&self, size: Option<Size>) {
let (width, height) = size.map(crate::extract_width_height).unzip();
let mut size_constraints = self.inner_size_constraints.borrow_mut();
let mut size_constraints = self.inner_size_constraints.write().unwrap();
size_constraints.min_width = width;
size_constraints.min_height = height;
self.set_size_constraints(*size_constraints)
}

pub fn set_max_inner_size(&self, size: Option<Size>) {
let (width, height) = size.map(crate::extract_width_height).unzip();
let mut size_constraints = self.inner_size_constraints.borrow_mut();
let mut size_constraints = self.inner_size_constraints.write().unwrap();
size_constraints.max_width = width;
size_constraints.max_height = height;
self.set_size_constraints(*size_constraints)
}

pub fn set_inner_size_constraints(&self, constraints: WindowSizeConstraints) {
*self.inner_size_constraints.borrow_mut() = constraints;
*self.inner_size_constraints.write().unwrap() = constraints;
self.set_size_constraints(constraints)
}

Expand Down Expand Up @@ -698,7 +701,7 @@ impl Window {
}

pub fn set_fullscreen(&self, fullscreen: Option<Fullscreen>) {
self.fullscreen.replace(fullscreen.clone());
*self.fullscreen.write().unwrap() = fullscreen.clone();
if let Err(e) = self
.window_requests_tx
.send((self.window_id, WindowRequest::Fullscreen(fullscreen)))
Expand All @@ -708,7 +711,7 @@ impl Window {
}

pub fn fullscreen(&self) -> Option<Fullscreen> {
self.fullscreen.borrow().clone()
self.fullscreen.read().unwrap().clone()
}

pub fn set_decorations(&self, decorations: bool) {
Expand Down Expand Up @@ -1025,7 +1028,7 @@ impl Window {
}

pub fn theme(&self) -> Theme {
if let Some(theme) = *self.preferred_theme.borrow() {
if let Some(theme) = *self.preferred_theme.read().unwrap() {
return theme;
}

Expand All @@ -1040,7 +1043,7 @@ impl Window {
}

pub fn set_theme(&self, theme: Option<Theme>) {
*self.preferred_theme.borrow_mut() = theme;
*self.preferred_theme.write().unwrap() = theme;
if let Err(e) = self
.window_requests_tx
.send((WindowId::dummy(), WindowRequest::SetTheme(theme)))
Expand Down
2 changes: 0 additions & 2 deletions src/platform_impl/macos/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,6 @@ pub(super) fn set_ns_theme(theme: Option<Theme>) {
}

struct WindowClass(&'static Class);
unsafe impl Send for WindowClass {}
unsafe impl Sync for WindowClass {}

static WINDOW_CLASS: Lazy<WindowClass> = Lazy::new(|| unsafe {
let window_superclass = class!(NSWindow);
Expand Down
Loading