Repository navigation
fix(minimald): log a requested session teardown at info - #1931
Merged
Merged
Conversation
A destroy SIGKILLs the session leader, and the reap that follows reported the same abnormal SIGKILL an OOM kill does, at warn. Every destroy wrote a line indistinguishable from a real unexpected death. The reap branch in Host::mainloop already knows whether the end was asked for: no pty error is stashed exactly when step erred on Message::Kill. Log that reap at info as "session process reaped after requested teardown", with the same fields, and keep the warn for every other death. Refs gominimal/inbox#838
Contributor
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 37 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
Comment |
Contributor
|
Approved: no findings at Nothing to do. Clobber: none: |
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.
What
Every
min session destroywrote a guest-log line that cannot be told apart from an OOM kill:A destroy sends
Message::Kill.Host::steplogssession host killed on requestat info, SIGKILLs the leader, and returnsErr.Host::mainloopthen takes thestep().is_err()branch, reaps the process, and always logged the reap atwarn!. hakoniwa reports the container SIGKILL with no exit code, so the line also carriedabnormal=true.That branch already knows whether the end was requested:
pending_pty_errisNoneexactly whensteperred onMessage::Kill. Nothing else returnsErrfromstepwithout first stashing a pty error.Change
mainlooprecordsrequested = pending.is_none()beforetake_ifcan emptypending.info!assession process reaped after requested teardown, with the same fields.warn!line, so an OOM kill or an external SIGKILL still stands out.ExitReason::is_abnormaland everything shown to the user are unchanged. Only the daemon log line changes.Tests
Two unit tests in
crates/minimald/src/session_host/tests.rs, both driving the mock host:a_requested_kill_logs_its_reap_at_info: aMessage::Killproduces one reap line, at INFO, with the requested-teardown message.an_unrequested_kill_still_warns_abnormal: an external SIGKILL of the live shell produces one reap line, at WARN, withabnormal=true.Refs gominimal/inbox#838