diff --git a/src/handlers/compositor.rs b/src/handlers/compositor.rs index f98a9824ef..daf342296f 100644 --- a/src/handlers/compositor.rs +++ b/src/handlers/compositor.rs @@ -498,7 +498,16 @@ impl CompositorHandler for State { .root_surface .retain(|k, v| k != surface && v != surface); - self.niri.dmabuf_pre_commit_hook.remove(surface); + // The object destruction order is not guaranteed to follow the logical role order. So for + // example when a client disconnects unexpectedly, WlSurface::destroyed() may be called + // before XdgShellHandler::toplevel_destroyed(). In this case, the surface will *not* have + // the default dmabuf pre-commit hook: it will still have the toplevel pre-commit hook. + // + // So, this may come out empty, and then the toplevel pre-commit hook will be removed in the + // subsequent toplevel_destroyed() call. + if let Some(hook) = self.niri.dmabuf_pre_commit_hook.remove(surface) { + remove_pre_commit_hook(surface, hook); + } } } @@ -517,6 +526,11 @@ delegate_shm!(State); impl State { pub fn add_default_dmabuf_pre_commit_hook(&mut self, surface: &WlSurface) { + if !surface.is_alive() { + error!("tried to add dmabuf pre-commit hook for a dead surface"); + return; + } + let hook = add_pre_commit_hook::(surface, move |state, _dh, surface| { let maybe_dmabuf = with_states(surface, |surface_data| { surface_data diff --git a/src/handlers/xdg_shell.rs b/src/handlers/xdg_shell.rs index 74cf5bc6a2..8512f85107 100644 --- a/src/handlers/xdg_shell.rs +++ b/src/handlers/xdg_shell.rs @@ -863,7 +863,15 @@ impl XdgShellHandler for State { self.niri.window_mru_ui.remove_window(id); self.niri.layout.remove_window(&window, transaction.clone()); - self.add_default_dmabuf_pre_commit_hook(surface.wl_surface()); + + let surface = surface.wl_surface(); + // This check is necessary because implicit resource destruction is done with + // undefined order, so the surface might get destroyed before toplevel_destroyed() is + // called. In this case, adding the default pre-commit hook here would leak it, since the + // place that removes it is WlSurface::destroyed(), which had already been called by now. + if surface.is_alive() { + self.add_default_dmabuf_pre_commit_hook(surface); + } // If this is the only instance, then this transaction will complete immediately, so no // need to set the timer.