-
Notifications
You must be signed in to change notification settings - Fork 920
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.
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
Application panics when reading Window::inner_size
after X server has terminated (e.g.: on user log-out)
#3686
Comments
Window::inner_size
after X server has terminatedWindow::inner_size
after X server has terminated
Window::inner_size
after X server has terminatedWindow::inner_size
after X server has terminated (e.g.: on user log-out)
Quick hack at trying to get that to compile (dunno if this is really a good idea - I'll let others decide this). Note that diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs
index ea7c0235..35b8635c 100644
--- a/src/platform_impl/linux/x11/window.rs
+++ b/src/platform_impl/linux/x11/window.rs
@@ -874,7 +874,7 @@ impl UnownedWindow {
})
}
- /// Refresh the API for the given monitor.
+ /// Refresh the DPI for the given monitor.
#[inline]
pub(super) fn refresh_dpi_for_monitor<T: 'static>(
&self,
@@ -885,7 +885,13 @@ impl UnownedWindow {
// Check if the self is on this monitor
let monitor = self.shared_state_lock().last_monitor.clone();
if monitor.name == new_monitor.name {
- let (width, height) = self.inner_size_physical();
+ let (width, height) = match self.inner_size_physical() {
+ Ok(size) => size,
+ Err(err) => {
+ tracing::error!("Cannot refresh DPI: {err:?}");
+ return;
+ }
+ };
let (new_width, new_height) = self.adjust_for_dpi(
// If we couldn't determine the previous scale
// factor (e.g., because all monitors were closed
@@ -1234,25 +1240,24 @@ impl UnownedWindow {
self.set_position_physical(x, y);
}
- pub(crate) fn inner_size_physical(&self) -> (u32, u32) {
+ pub(crate) fn inner_size_physical(&self) -> Result<(u32, u32), X11Error> {
// This should be okay to unwrap since the only error XGetGeometry can return
// is BadWindow, and if the window handle is bad we have bigger problems.
self.xconn
.get_geometry(self.xwindow)
.map(|geo| (geo.width.into(), geo.height.into()))
- .unwrap()
}
#[inline]
pub fn inner_size(&self) -> PhysicalSize<u32> {
- self.inner_size_physical().into()
+ self.inner_size_physical().unwrap_or((1, 1)).into()
}
#[inline]
pub fn outer_size(&self) -> PhysicalSize<u32> {
let extents = self.shared_state_lock().frame_extents.clone();
if let Some(extents) = extents {
- let (width, height) = self.inner_size_physical();
+ let (width, height) = self.inner_size().into();
extents.inner_size_to_outer(width, height).into()
} else {
self.update_cached_frame_extents(); |
I would think that shutting down the X server would cause the application termination event to occur. Though I guess I haven't tested this. Otherwise I would say that this error is expected; using a window after the |
At the next breaking change we should make any method that communicates with the X server fallible. |
See #3317. We have discussed this so many times now, I kinda lost sight of all the crazy solutions we came up with to solve this. I would prefer to find a different solution then returning a |
Description
Our application's error reporting has been capturing a lot of
called Result::unwrap() on an Err value: Connection(IoError(Custom { kind: Other, error: UnknownError }))
panics, stemming from the unwrap inWindow::inner_size_physical
.We finally have figured out the cause:
Window::inner_size
.winit
attempts to get the window geometry, and receives a connection error, as the display server is gone.winit
panicsXlib traditionally would end up invoking
exit()
here as part of the default I/O (fatal) error handler.winit
's use ofxcb
makes it responsible, instead, for handling connection errors.My basic request here is that
winit
doesn't panic in this situation and instead handles it in any more graceful way (could mean callingexit()
like Xlib would have). This is an expected situation, and should be handled accordingly.I think that in an ideal world (but curious what
winit
devs think),Window::inner_size
returns aResult
, allowing it to tell the caller that the size couldn't be computed, andwinit
then terminates the event loop at its earliest convenience, allowing for graceful application shutdown.OS and window mananger
Arch Linux; Gnome (either X11 or Wayland w/ application running via Xwayland).
Winit version
0.30.0
The text was updated successfully, but these errors were encountered: