Skip to content
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

Don't make X11 requests to a destroyed window. #1103

Merged
merged 2 commits into from
Jul 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ You can find its changes [documented below](#060---2020-06-01).
- X11: Support timers. ([#1096] by [@psychon])
- `EnvScope` now also updates the `Env` during `Widget::lifecycle`. ([#1100] by [@finnerale])
- `WidgetExt::debug_widget_id` and `debug_paint_layout` now also apply to the widget they are called on. ([#1100] by [@finnerale])
- X11: Fix X11 errors caused by destroyed windows ([#1103] by [@jneem])

### Visual

Expand Down Expand Up @@ -368,6 +369,7 @@ Last release without a changelog :(
[#1096]: https://github.com/linebender/druid/pull/1096
[#1097]: https://github.com/linebender/druid/pull/1097
[#1100]: https://github.com/linebender/druid/pull/1100
[#1103]: https://github.com/linebender/druid/pull/1103

[Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master
[0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0
Expand Down
37 changes: 34 additions & 3 deletions druid-shell/src/platform/x11/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

use std::any::Any;
use std::cell::RefCell;
use std::convert::{TryFrom, TryInto};
use std::collections::BinaryHeap;
use std::convert::{TryFrom, TryInto};
use std::os::unix::io::RawFd;
use std::rc::{Rc, Weak};
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -254,6 +254,7 @@ impl WindowBuilder {
let state = RefCell::new(WindowState {
size: self.size,
invalid: Rect::ZERO,
destroyed: false,
});
let present_data = match self.initialize_present_data(id) {
Ok(p) => Some(p),
Expand Down Expand Up @@ -442,6 +443,8 @@ struct WindowState {
size: Size,
/// The region that was invalidated since the last time we rendered.
invalid: Rect,
/// We've told X11 to destroy this window, so don't so any more X requests with this window id.
destroyed: bool,
}

/// A collection of pixmaps for rendering to. This gets used in two different ways: if the present
Expand Down Expand Up @@ -510,7 +513,17 @@ impl Window {

/// Start the destruction of the window.
pub fn destroy(&self) {
log_x11!(self.app.connection().destroy_window(self.id));
if !self.destroyed() {
match borrow_mut!(self.state) {
Ok(mut state) => state.destroyed = true,
Err(e) => log::error!("Failed to set destroyed flag: {}", e),
}
log_x11!(self.app.connection().destroy_window(self.id));
}
}

fn destroyed(&self) -> bool {
borrow!(self.state).map(|s| s.destroyed).unwrap_or(false)
}

fn size(&self) -> Result<Size, Error> {
Expand Down Expand Up @@ -568,6 +581,10 @@ impl Window {
}

fn render(&self) -> Result<(), Error> {
if self.destroyed() {
return Ok(());
}

let size = borrow!(self.state)?.size;
let invalid_rect = borrow!(self.state)?.invalid;
let mut anim = false;
Expand Down Expand Up @@ -634,7 +651,9 @@ impl Window {
}

fn show(&self) {
log_x11!(self.app.connection().map_window(self.id));
if !self.destroyed() {
log_x11!(self.app.connection().map_window(self.id));
}
}

fn close(&self) {
Expand All @@ -653,6 +672,10 @@ impl Window {

/// Bring this window to the front of the window stack and give it focus.
fn bring_to_front_and_focus(&self) {
if self.destroyed() {
return;
}

// TODO(x11/misc): Unsure if this does exactly what the doc comment says; need a test case.
let conn = self.app.connection();
log_x11!(conn.configure_window(
Expand Down Expand Up @@ -723,6 +746,10 @@ impl Window {
}

fn set_title(&self, title: &str) {
if self.destroyed() {
return;
}

// This is technically incorrect. STRING encoding is *not* UTF8. However, I am not sure
// what it really is. WM_LOCALE_NAME might be involved. Hopefully, nothing cares about this
// as long as _NET_WM_NAME is also set (which uses UTF8).
Expand Down Expand Up @@ -948,6 +975,10 @@ impl Window {
}

pub fn handle_idle_notify(&self, event: &IdleNotifyEvent) -> Result<(), Error> {
if self.destroyed() {
return Ok(());
}

let mut buffers = borrow_mut!(self.buffers)?;
if buffers.all_pixmaps.contains(&event.pixmap) {
buffers.idle_pixmaps.push(event.pixmap);
Expand Down