Skip to content

moveHandler: Remove the monitor grace period timer when a grab ends - #461

Open
PhilMeyr wants to merge 1 commit into
ubuntu:mainfrom
PhilMeyr:fix/monitor-grace-period-timer-crash
Open

moveHandler: Remove the monitor grace period timer when a grab ends#461
PhilMeyr wants to merge 1 commit into
ubuntu:mainfrom
PhilMeyr:fix/monitor-grace-period-timer-crash

Conversation

@PhilMeyr

Copy link
Copy Markdown

Fixes the GNOME Shell abort reported in #435, which several of us have hit on Ubuntu 26.04 (@YannickMG, @cpaelzer and myself, plus the original report on Debian 13 / GNOME 48).

The bug

_edgeTilingPreview() arms a 150 ms grace period timer when a drag crosses a monitor boundary, so the tile preview sticks to the previous monitor for a moment:

this._latestMonitorLockTimerId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 150, () => {
    if (timerId === this._latestMonitorLockTimerId) {
        this._monitorNr = global.display.get_current_monitor();
        if (global.display.is_grabbed())
            this._edgeTilingPreview(window, grabOp);
    }
    ...

The callback captures window, but the source is only ever removed in destroy() — never when the grab it belongs to ends. _onMoveFinished()'s finally block tears down everything else (_posChangedId, the tile preview, _isGrabOp) and leaves this one pending.

So if the dragged window is destroyed within those 150 ms — tearing off or dropping a browser tab does exactly that — the orphaned timer still fires and re-enters _edgeTilingPreview(), reaching:

const workArea = new Rect(window.get_work_area_for_monitor(this._monitorNr));

meta_window_get_work_area_for_monitor()meta_window_get_workspaces()g_assert_not_reached() on an unmanaging window → SIGABRT. Being a C assertion, no JS guard can contain it, and the whole Wayland session dies.

The unmanaging JS error logged from _onMoveFinished() in every report is a separate, non-fatal event that merely signals the window is gone. In my logs the two are consistently ~140 ms apart, and on 2026-07-28 that error occurred with no abort at all — the session survived two more days.

The fix

  • Remove the pending source in the finally block of _onMoveFinished(), so it cannot outlive the operation that armed it. This is what actually fixes the crash.
  • Additionally guard the callback with window.get_compositor_private(). global.display.is_grabbed() is a property of the display and says nothing about this window still being managed; the same idiom is already used in focusHint.js:290.

Notes

  • Only the grace period path is affected, so monitor-switch-grace-period false works as a user-side workaround on released versions.
  • The timer is armed exclusively inside if (this._lastMonitorNr !== currMonitorNr), which matches every report in Wayland crash report #435 being multi-monitor.
  • I could not build a deterministic reproducer — it is a race against a 150 ms window — so this is reasoned from four crash traces rather than from a red-to-green test. Happy to adjust if you would rather keep only the finally cleanup and drop the second guard.

When a drag crosses a monitor boundary, _edgeTilingPreview() arms a 150 ms
timer that captures the dragged window and re-enters _edgeTilingPreview()
when it fires. Nothing removes that source when the grab ends, so a window
destroyed within those 150 ms leaves it pending. It then reaches
window.get_work_area_for_monitor(), which asserts in
meta_window_get_workspaces() on an unmanaging window and aborts the shell.

Remove the pending source in the finally block of _onMoveFinished(), and
guard the callback with get_compositor_private(), since is_grabbed() is a
property of the display rather than of the window.

Closes ubuntu#435
@PhilMeyr PhilMeyr mentioned this pull request Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant