Repository navigation
Close the window when Run returns, and fail loudly when Spec.Tray cannot attach - #29
Merged
Merged
Conversation
…ut a tray Run never closed its window back-end. A back-end's Run returns when the window is asked to close, and on X11 that leaves the window mapped, frozen, with its connection open, until a garbage collection finalises it. claimward-vpn-app-linux closed it through the only reference Run left, the toolkit clipboard. run now closes the window when its loop returns, and restores the toolkit clipboard it installed from it. Cocoa is the exception: its loop has already closed the window, and a Close sent afterwards is queued on the main run loop and delivered to the next window's loop instead. Spec.Tray silently gave no tray where the tray backend cannot attach (Linux and Windows, before go-widgets/tray#36): the attach error was swallowed. Run now closes the window and returns that error, wrapped. The window is opened through a seam (openWindow), so the lifecycle around the loop is tested against a fake back-end; run takes a quit channel Run uses to close the window when the attach fails. Fixes #27 Fixes #28 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
tray v0.14.0 (go-widgets/tray#36) implements Attach on Linux and Windows, where it used to return ErrNoBackend. With this, Spec.Tray works there instead of returning the error #28 now surfaces. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
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.
Fixes #27
Fixes #28
#27: Run never closed the window back-end
A back-end's
Runreturns when its window is asked to close, and on X11 that leaves the window mapped, frozen, with its connection open, until a GC finalises it. claimward-vpn-app-linux works around it withview.CloseLeftoverWindow, closing the window through the only referenceRunleft behind (the toolkit clipboard).Fix:
runcloses the window when its loop returns (defer closeAfterRun(win)), and restores the toolkit clipboard it installed from the window, which would otherwise outlive it.Cocoa exception (
closedByItsOwnLoop): its loop returns only once the window has already closed, andCloseis performed on the main thread. Sent after the loop stopped, it waits in the run-loop queue and gets delivered to the next window's loop.viewCloseNowacts on the active window, so it would close the window an app opens next. Win32'sCloseafterWM_DESTROYis already a no-op (hwnd is 0).#28: Spec.Tray silently gave no tray on Linux
attachTrayswallowedtray.Attach's error, and the Linux (and Windows) tray backends do not implementAttach. Now, when the attach fails,Runcloses the window (through a newquitchannelrunlistens on) and returnsapplication: Spec.Tray could not be attached: <err>;errors.Is(err, tray.ErrNoBackend)works. go-widgets/tray#36 implementsAttachon Linux and Windows. With that release,Spec.Trayworks there; before it, the same code gives this error instead of nothing.Tests (
lifecycle_test.go, via a newopenWindowseam and a fake back-end)TestRunClosesTheWindowWhenItsLoopReturns,TestRunPutsTheClipboardBack,TestRunLeavesAWindowThatClosedItselfAlone,TestRunReportsAWindowThatWouldNotOpen,TestRunQuitClosesTheWindowTestRunFailsLoudlyWhenTheTrayCannotAttach: realrun, fake window that stays open,attachTrayfailing.Runmust return the wrapped error and close the window.runsignature.TestAttachTrayDefaultIsHarmlessis nowTestAttachTrayDefaultReportsAFailure.run(which is now partly exercised too).-raceclean on linux and darwin.Sabotage checks (each sabotaged tree passes
go vet):closeAfterRun-> "the window was closed 0 times after its loop returned, want 1"Live check on Ubuntu 24.04 arm64 (Xvfb + openbox + a D-Bus session). A probe calls
application.Run, closes the window withwmctrl -c(WM_DELETE_WINDOW, as the close button sends), and checksxdotool search --onlyvisible2 s afterRunreturns:Runreturns nilRunreturns "Spec.Tray could not be attached: tray: no backend for this platform"Runreturns nilNot verified
Closequeued after one window's loop closed the next window the moment it opened. That PR fixes the hazard in the window itself (the main-thread handlers check the receiving view). Once a window release has it,closedByItsOwnLoopis only belt-and-braces.application.Runitself was not run on macOS.Closeafter the loop is a no-op by reading.Release order
github.com/go-widgets/trayto that release (go get github.com/go-widgets/tray@v0.14.0 && go mod tidy) before merging, soSpec.Trayworks on Linux/Windows instead of failing. The PR builds as it is against the released tray (go.mod untouched), and failing loudly there is still correct.go-widgets/window#135 (Show/Hide/Raise) is independent: this PR neither needs nor uses it.
Semver: minor (v0.x+1.0). There is no new exported identifier, but
Runnow returns an error where it used to return nil (aSpec.Traythat cannot attach), and callers can see that.🤖 Generated with Claude Code