diff --git a/src/backends/wayland/mod.rs b/src/backends/wayland/mod.rs index 4fe9f299..1c85e0db 100644 --- a/src/backends/wayland/mod.rs +++ b/src/backends/wayland/mod.rs @@ -22,7 +22,7 @@ struct State; #[derive(Debug)] pub struct WaylandDisplayImpl { - conn: Option, + conn: Connection, event_queue: Mutex>, qh: QueueHandle, shm: wl_shm::WlShm, @@ -30,14 +30,10 @@ pub struct WaylandDisplayImpl { /// The object that owns the display handle. /// /// This has to be dropped *after* the `conn` field, because the `conn` field implicitly borrows - /// this. + /// this. This is done by placing this field last, see: + /// _display: D, -} - -impl WaylandDisplayImpl { - fn conn(&self) -> &Connection { - self.conn.as_ref().unwrap() - } + // NO FIELDS AFTER THIS! } impl ContextInterface for Arc> { @@ -64,7 +60,7 @@ impl ContextInterface for Arc Drop for WaylandDisplayImpl { if self.shm.version() >= 2 { self.shm.release(); } - // Make sure the connection is dropped first. - self.conn = None; } } #[derive(Debug)] -pub struct WaylandImpl { - display: Arc>, - surface: Option, +pub struct WaylandImpl { + surface: wl_surface::WlSurface, buffers: Option<(WaylandBuffer, WaylandBuffer)>, size: Option<(NonZeroI32, NonZeroI32)>, /// The pointer to the window object. /// /// This has to be dropped *after* the `surface` field, because the `surface` field implicitly - /// borrows this. + /// borrows this. This is done by placing this field last, see: + /// window_handle: W, + /// Drop the display last. + display: Arc>, + // NO FIELDS AFTER THIS! } impl SurfaceInterface @@ -120,11 +117,11 @@ impl SurfaceInterface ) } .swbuf_err("Failed to create proxy for surface ID.")?; - let surface = wl_surface::WlSurface::from_id(display.conn(), surface_id) + let surface = wl_surface::WlSurface::from_id(&display.conn, surface_id) .swbuf_err("Failed to create proxy for surface ID.")?; Ok(Self { display: display.clone(), - surface: Some(surface), + surface, buffers: Default::default(), size: None, window_handle: window, @@ -221,7 +218,7 @@ impl SurfaceInterface let age = back.age; Ok(BufferImpl { event_queue: &self.display.event_queue, - surface: self.surface.as_ref().unwrap(), + surface: &self.surface, front, back, width, @@ -231,13 +228,6 @@ impl SurfaceInterface } } -impl Drop for WaylandImpl { - fn drop(&mut self) { - // Make sure the surface is dropped first. - self.surface = None; - } -} - #[derive(Debug)] pub struct BufferImpl<'surface> { event_queue: &'surface Mutex>, diff --git a/src/backends/x11.rs b/src/backends/x11.rs index 3a6c454b..741d1410 100644 --- a/src/backends/x11.rs +++ b/src/backends/x11.rs @@ -40,7 +40,7 @@ use x11rb::xcb_ffi::XCBConnection; #[derive(Debug)] pub struct X11DisplayImpl { /// The handle to the XCB connection. - connection: Option, + connection: XCBConnection, /// SHM extension is available. is_shm_available: bool, @@ -53,7 +53,11 @@ pub struct X11DisplayImpl { /// Without `&mut`, the underlying connection cannot be closed without other unsafe behavior. /// With `&mut`, the connection can be dropped without us knowing about it. Therefore, we /// cannot provide `&mut` access to this field. + /// + /// This has to be dropped *after* the `conn` field. This is done by placing this field last: + /// _display: D, + // NO FIELDS AFTER THIS! } impl ContextInterface for Arc> { @@ -110,7 +114,7 @@ impl ContextInterface for Arc let supported_visuals = supported_visuals(&connection); Ok(Arc::new(X11DisplayImpl { - connection: Some(connection), + connection, is_shm_available, supported_visuals, _display: display, @@ -118,20 +122,9 @@ impl ContextInterface for Arc } } -impl X11DisplayImpl { - fn connection(&self) -> &XCBConnection { - self.connection - .as_ref() - .expect("X11DisplayImpl::connection() called after X11DisplayImpl::drop()") - } -} - /// The handle to an X11 drawing context. #[derive(Debug)] -pub struct X11Impl { - /// X display this window belongs to. - display: Arc>, - +pub struct X11Impl { /// The window to draw to. window: xproto::Window, @@ -154,7 +147,13 @@ pub struct X11Impl { size: Option<(NonZeroU16, NonZeroU16)>, /// Keep the window alive. + /// NOTE: Drop this second-to-last! window_handle: W, + + /// X display this window belongs to. + /// NOTE: Drop this last! + display: Arc>, + // NO FIELDS AFTER THIS! } /// The buffer that is being drawn to. @@ -219,13 +218,13 @@ impl SurfaceInterface fo let display2 = display.clone(); let tokens = { let geometry_token = display2 - .connection() + .connection .get_geometry(window) .swbuf_err("Failed to send geometry request")?; let window_attrs_token = if window_handle.visual_id.is_none() { Some( display2 - .connection() + .connection .get_window_attributes(window) .swbuf_err("Failed to send window attributes request")?, ) @@ -238,11 +237,11 @@ impl SurfaceInterface fo // Create a new graphics context to draw to. let gc = display - .connection() + .connection .generate_id() .swbuf_err("Failed to generate GC ID")?; display - .connection() + .connection .create_gc( gc, window, @@ -344,7 +343,7 @@ impl SurfaceInterface fo if self.size != Some((width, height)) { self.buffer_presented = false; self.buffer - .resize(self.display.connection(), num_bytes) + .resize(&self.display.connection, num_bytes) .swbuf_err("Failed to resize X11 buffer")?; // We successfully resized the buffer. @@ -358,11 +357,11 @@ impl SurfaceInterface fo tracing::trace!("next_buffer: window={:X}", self.window); // Finish waiting on the previous `shm::PutImage` request, if any. - self.buffer.finish_wait(self.display.connection())?; + self.buffer.finish_wait(&self.display.connection)?; // We can now safely call `next_buffer` on the buffer. Ok(BufferImpl { - connection: self.display.connection(), + connection: &self.display.connection, window: self.window, gc: self.gc, depth: self.depth, @@ -382,7 +381,7 @@ impl SurfaceInterface fo // TODO: Is it worth it to do SHM here? Probably not. let reply = self .display - .connection() + .connection .get_image( xproto::ImageFormat::Z_PIXMAP, self.window, @@ -775,14 +774,7 @@ impl Drop for ShmSegment { } } -impl Drop for X11DisplayImpl { - fn drop(&mut self) { - // Make sure that the x11rb connection is dropped before its source is. - self.connection = None; - } -} - -impl Drop for X11Impl { +impl Drop for X11Impl { fn drop(&mut self) { // If we used SHM, make sure it's detached from the server. if let Buffer::Shm(mut shm) = mem::replace( @@ -790,10 +782,10 @@ impl Drop for X11Impl { Buffer::Wire(util::PixelBuffer(Vec::new())), ) { // If we were in the middle of processing a buffer, wait for it to finish. - shm.finish_wait(self.display.connection()).ok(); + shm.finish_wait(&self.display.connection).ok(); if let Some((segment, seg_id)) = shm.seg.take() { - if let Ok(token) = self.display.connection().shm_detach(seg_id) { + if let Ok(token) = self.display.connection.shm_detach(seg_id) { token.ignore_error(); } @@ -803,7 +795,7 @@ impl Drop for X11Impl { } // Close the graphics context that we created. - if let Ok(token) = self.display.connection().free_gc(self.gc) { + if let Ok(token) = self.display.connection.free_gc(self.gc) { token.ignore_error(); } }