image-copy: Fix panics on capture failure paths - #2743
Open
minyek wants to merge 1 commit into
Open
Conversation
render_workspace_to_buffer's constraints-failure path removed the session from the workspace's Output, but workspace-scope sessions live in Workspace::image_copy. On an output that never hosted an output-scope capture, the ImageCopySessionsData unwrap panicked — a workspace-scope screencast frame arriving while the output has no current mode crashed the compositor. On an output that had, the retain() was a silent no-op: the workspace's owned Session was never dropped, so the client never received `stopped` and kept submitting frames to the dead session, each failing as `unknown`. Remove from the workspace instead: dropping the owned Session stops the session and fails the frame as `stopped`. Harden the remaining paths of the same shape: - SessionHolder::remove_session/remove_cursor_session on Output and CosmicSurface tolerate a missing ImageCopySessionsData instead of unwrapping, making removal idempotent. - constraints_for_output/constraints_for_toplevel no longer unwrap offscreen_renderer(); renderer-creation failure (e.g. after a GPU reset) now fails the capture instead of panicking, matching the render-path callers. Developed with AI assistance (Claude Code); all changes have been reviewed and are understood by the author.
Drakulix
reviewed
Aug 17, 2026
| }) | ||
| .unwrap(); | ||
| .inspect_err(|err| tracing::warn!(?err, "Couldn't use node for screencopy")) | ||
| .ok()?; |
Member
There was a problem hiding this comment.
We shouldn't run into a situation where we have an invalid node for a given output since the renderer is created on the main thread. This points to a deeper issue we should fix instead. Ping @ids1024
| }) | ||
| .unwrap(); | ||
| .inspect_err(|err| tracing::warn!(?err, "Couldn't use node for screencopy")) | ||
| .ok()?; |
| { | ||
| workspace.remove_session(session); | ||
| } | ||
| frame.fail(CaptureFailureReason::Stopped); |
Member
There was a problem hiding this comment.
I don't think we need to explicitly call fail, if we drop the session.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two compositor-crash panics and a stuck-session bug in the
image-copy-capture failure handling.
Workspace captures: session removed from the wrong holder.
render_workspace_to_buffer's constraints-failure path calledoutput.remove_session(), but workspace-scope sessions live inWorkspace::image_copy. If the output never hosted an output-scope capture,the unguarded
ImageCopySessionsDataunwrap panicked — reachable by anyworkspace screencast frame arriving while the output has no current mode
(e.g. it is being unplugged or disabled). Otherwise the
retain()was asilent no-op: the workspace's owned
Sessionwas never dropped, so theclient never received
stoppedand kept submitting frames to a deadsession, each failing as
unknown. The session is now removed from theworkspace; dropping it stops the session and fails the frame as
stopped.Unguarded holder unwraps.
remove_session/remove_cursor_sessiononOutputandCosmicSurfacenow tolerate a missingImageCopySessionsData,making removal idempotent (matching the existing
sessions()/remove_framestyle).
Renderer-creation failure during constraint negotiation.
constraints_for_output/constraints_for_toplevelunwrappedoffscreen_renderer(), which can fail (e.g. after a GPU reset — a case therender-path callers already handle gracefully since 9514b49). They now fail
the capture instead of panicking.
Testing
The failure paths were verified against smithay's session/frame lifecycle:
dropping the workspace's owned
Sessionfails its active frames asstoppedand sends
stopped, and aNonefrom the constraints functions produces astopped session at creation and is handled by the existing else-branches in
the render-path callers. Normal capture behaviour is unchanged — the diff
only touches failure paths.
AI assistance
This work was developed with AI assistance (Claude Code); use of AI-generated
code is disclosed in the commit messages per the contribution guidelines. All
changes have been reviewed and are understood by the author.
Checklist