Skip to content

fix: guard tree rendering against destroyed window actors #507

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.

Sign up for GitHub

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

Open
braceyourself wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −5
Open

fix: guard tree rendering against destroyed window actors #507

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 39 additions & 5 deletions lib/extension/tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,9 @@ export class Node extends GObject.Object {
style_class: "window-tabbed-tab",
x_expand: true,
});
// Window tracker may not resolve an app for a dying window
let app = this.app;
if (!app) return;
let labelText = this._getTitle();
let metaWin = this.nodeValue;
let titleButton = new St.Button({
Expand Down Expand Up @@ -537,6 +539,21 @@ export class Node extends GObject.Object {
return null;
}

// Check if the underlying window actor is still alive. GJS throws
// on property access of finalized GObjects rather than segfaulting,
// so a cheap get_name() call is enough to detect dead actors.
isNodeValid() {
if (!this.isWindow()) return true;
try {
let actor = this._actor;
if (!actor) return false;
actor.get_name();
return true;
} catch (e) {
return false;
}
}

render() {
// Always update the title for the tab
if (this.tab !== null && this.tab !== undefined) {
Expand Down Expand Up @@ -1274,8 +1291,12 @@ export class Tree extends Node {
tiledChildren.forEach((w) => {
if (w.renderRect) {
if (w.renderRect.width > 0 && w.renderRect.height > 0) {
// Window may have been destroyed since processNode computed renderRect
let metaWin = w.nodeValue;
this.extWm.move(metaWin, w.renderRect);
try {
this.extWm.move(metaWin, w.renderRect);
} catch (e) {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply() now swallows all exceptions from this.extWm.move() with an empty catch. This makes unrelated failures (bad rects, API changes, etc.) silent and hard to diagnose. If the intention is to ignore “window destroyed mid-render” errors, consider (a) filtering invalid windows before calling move and/or (b) logging at least a debug/warn message (possibly rate-limited) inside the catch so unexpected errors are visible.

Suggested change
} catch (e) {
} catch (e) {
Logger.debug(
`extWm.move failed for window ${metaWin} with rect ${JSON.stringify(
w.renderRect
)}: ${e}`
);

Copilot uses AI. Check for mistakes.
}
} else {
Logger.debug(`ignoring apply for ${w.renderRect.width}x${w.renderRect.height}`);
}
Expand Down Expand Up @@ -1385,7 +1406,8 @@ export class Tree extends Node {
});
}

tiledChildren.forEach((child, index) => {
// Skip windows whose actors were destroyed mid-render
tiledChildren.filter((c) => c.isNodeValid()).forEach((child, index) => {
// A monitor can contain a window or container child
if (node.layout === LAYOUT_TYPES.HSPLIT || node.layout === LAYOUT_TYPES.VSPLIT) {
Comment on lines +1409 to 1412
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In processNode(), sizes is computed from the unfiltered tiledChildren, but the later loop filters with tiledChildren.filter((c) => c.isNodeValid()) and uses the filtered loop’s index to read params.sizes[index]. If any invalid nodes are filtered out, the indices no longer line up and split/tab/stack calculations will apply the wrong sizes to the remaining windows (and can leave stale decoration state when the filtered list becomes empty). Filter invalid nodes before computing sizes and set params.tiledChildren to that same filtered list so indexing and decoration logic stay consistent.

Copilot uses AI. Check for mistakes.
this.processSplit(node, child, params, index);
Expand Down Expand Up @@ -1527,10 +1549,17 @@ export class Tree extends Node {
if (node.childNodes.length > 1 || alwaysShowDecorationTab) {
nodeY = nodeRect.y + params.stackedHeight;
nodeHeight = nodeRect.height - params.stackedHeight;
if (node.decoration && child.isWindow()) {
if (node.decoration && child.isWindow() && child.isNodeValid()) {
let gap = this.extWm.calculateGaps(node);
let renderRect = this.processGap(node);
let borderWidth = child.actor.border.get_theme_node().get_border_width(St.Side.TOP);
// Border actor may be gone if the window was destroyed mid-render
let borderWidth = 0;
try {
if (child.actor?.border) {
borderWidth = child.actor.border.get_theme_node().get_border_width(St.Side.TOP);
}
} catch (e) {
}
Comment on lines +1555 to +1562
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try/catch around child.actor.border.get_theme_node() has an empty catch block. If this is expected when windows are destroyed mid-render, please add at least a brief comment or debug-level log in the catch so unexpected theme/actor errors don’t get silently ignored (and consider narrowing the guarded code to only what can throw).

Copilot uses AI. Check for mistakes.

// Make adjustments to the gaps
let adjust = 4 * Utils.dpi();
Expand All @@ -1552,7 +1581,12 @@ export class Tree extends Node {
} else {
decoration.hide();
}
if (!decoration.contains(child.tab)) decoration.add_child(child.tab);
if (child.tab && !decoration.contains(child.tab)) {
try {
decoration.add_child(child.tab);
} catch (e) {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try/catch around decoration.add_child(child.tab) currently ignores all errors with an empty catch. This can hide real UI issues (e.g., actor already disposed for reasons other than window destruction). Consider logging at debug/warn (or adding a clarifying comment) so unexpected failures are diagnosable, or restructure to avoid calling add_child when the tab actor is no longer valid.

Suggested change
} catch (e) {
} catch (e) {
Logger.debug(`Failed to add tab actor to decoration: ${e}`);

Copilot uses AI. Check for mistakes.
}
}
}

child.render();
Expand Down
Loading