Make safe WindowHandle s to imilictly ref-count #158
Description
The current handle can only work safely when the window is either end up in OnceCell, done Box::leak, or send in some sort of wrapper to other thread. However the most common way write multi window is doing something like that
struct State {
windows: HashMap<WindowId, MyWindow>,
}
struct MyWindow {
window: Window
surface: Surface, // depends on Window. Explicit bug here.
}Such state is being owned by EventLoop or defined as part of the captured value in the closure, so it can't have correct lifetime. Rust doesn't allow to have Surface depend on Window and be stored in the same struct, but just in the right order.
This could can work, but with e.g. GBM/drm stuff it may segfault, for example, see Smithay/smithay#1102 as an example.
One solution could be to have a WindowHandle even owned one somehow retain the original object so it can't drop in the wrong order.
This can work really nice when the Window itself is not owned by the user, but rather by the library, so to remove the Window they'll have to call failing destructor and the library could just deny you dropping it because you still hold reference to it.
Such design doesn't really need a lifetime, since you just can't drop the original object. Yes it does require the library serving the handle to retain the resources, but it's safe.
Even if the handle gets invalidated, it's often safe to use, so it's not an issue and irrelevant to the suggestion.
Some sketch of the API and semantics based on the rust-windowing/winit#3367 .
pub struct State {
surface: Option<glutin::Surface>,
}
impl Application for State {
fn new_events(&mut self, loop_handle: &mut dyn EventLoopHandle, start_cause: StartCause) {
let _ = loop_handle.create_window(&Default::default());
}
fn about_to_wait(&mut self, _: &mut dyn EventLoopHandle) {
println!("About to wait");
}
fn loop_exiting(&mut self, _: &mut dyn EventLoopHandle) {
println!("Exiting the loop");
}
}
impl ApplicationWindow for State {
fn created(&mut self, loop_handle: &mut dyn EventLoopHandle, window_id: WindowId) {
let window = loop_handle.get_window(window_id).unwrap();
// This retain the original window inside the owned event loop.
let handle = window.window_handle();
// This call is actually safe now, because it can't outlive the window no matter how you
// write it, since iti retains the original one.
self.surface = Some(glutin::Surface::new(&window).expect("failed to create glutin surface"));
}
fn close_requested(&mut self, loop_handle: &mut dyn EventLoopHandle, window_id: WindowId) {
// Surface retained the `Window` inside the winit, so it can't drop it and will throw
// an error until you actually destroy it.
//self.surface = None;
// The window got a close request. Try to close it without dropping the `Surface`.
// This line will panic.
loop_handle.remove_window(&window_id).except("failed to destroy widndow because it has
handles holding it");
}
fn scale_factor_changed(
&mut self,
_: &mut dyn EventLoopHandle,
_: WindowId,
scale_factor: f64,
) {
println!("New scale factor {scale_factor}");
}
fn redraw_requested(&mut self, loop_handle: &mut dyn EventLoopHandle, window_id: WindowId) {
let (window, surface) = match (loop_handle.get_window(window_id), self.surface.as_mut()) {
(Some(window), Some(surface)) => (window, surface),
_ => return,
};
surface.swap_buffers();
}
fn destroyed(&mut self, loop_handle: &mut dyn EventLoopHandle, _: WindowId) {
if loop_handle.num_windows() == 0 {
loop_handle.exit();
}
}
}
fn main() {
let mut event_loop = <EventLoop as EventLoopRequests<State>>::new().unwrap();
let context =
unsafe { Context::new(&event_loop).expect("failed to create softbuffer context") };
let state = State { context, surface: None };
let proxy = EventLoopRequests::<State>::proxy(&mut event_loop);
// Test out the proxy.
std::thread::spawn(move || loop {
proxy.wakeup();
std::thread::sleep(Duration::from_millis(500));
});
event_loop.run(state);
}If we state that to safely impl the window must be dropped only when it has only a single reference(from itself) it means that it can't also outlive the event loop, since it'll continue to run until all the windows are closed.
So the end API could be safe in all cases, unlike the current one where you're forced to store data in a special way due to lifetime.