Repository navigation
Quit the installed macOS tray as macOS does, a logout's quit among them - #1031
Merged
Merged
Conversation
The tray-install job started the tray and waited for it to listen, and nothing ever asked it to quit. On macOS it now sends the tray the quit Apple events by pid: an ordinary one, which the tray refuses (-128) and turns into its own exit, and a logout's, kAEQuitReason set to kAELogOut, which it has to let through. Each has to end the tray with status 0 and its version marker gone. CI cannot log out; the event is what a logout sends, so the whole of TerminateGuard's session path runs.
On macOS a connection to 3492 was taken before the installed tray had started, so the install step never waited for it, and the quit events went to a tray that was still starting. Ask lsof whether the tray's own process id is listening, and list whatever else holds the port first.
send-quit set kAEQuitReason as a parameter of the quit event, and TerminateGuard reads it as an attribute, so the logout's quit read as an ordinary one and was refused. The install step's connection check was never wrong: the tray listens within a second, and its first line of output only arrives late because it is buffered. So it is back as it was, and only the quit step asks lsof, for the process the events go to.
A tray that starts and quits cleanly logs nothing, so there is no log.txt, and under bash -e the cat of it ended the step that had just passed, and would have ended each failing path before its error line.
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.
This adds the lifecycle coverage the review of #991 asked for.
Until now the
tray-installjob started the installed tray, waited for it to listen on 3492 and then killed it. Nothing ever asked it to quit the way macOS does.What's new. On macOS, a new step starts the installed tray twice and quits it with an Apple event each time, sent by pid from a small Swift script (
.github/scripts/send-quit.swift, compiled withxcrun swiftc).kAEQuitReasonset tokAELogOut. It must not be answered -128, because that refusal is what cancels a logout.$TMPDIR/DiffEngineTray/version.txt). After a logout's quit, only the session end can remove the marker, because AppKit ends the process without going through the loop's own exit.CI can't log out, but the event is what a logout sends, so the whole of
TerminateGuard's session path runs. That covers reading the reason, calling back into managed code throughdetray_on_session_end, and returning.terminateNow.Not covered. No inline snapshot is in the queue, so this doesn't show a trio being staged. Getting one into the tray's queue means driving a client, and the tray then opens a viewer window.
TraySessionEndis shared with every other way a session ends, and its own tests cover staging.Tests. Nothing here can be run on Windows; this PR's macOS
tray-installjob is the test. It shows the following:The logout reason has to go on the event as an attribute, which is where loginwindow puts it. A first version of the script sent it as a parameter, and the tray rightly took that for an ordinary quit.