Repository navigation
Conversation
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.
Problem and reproduction
Closing a shell can make a running agent unresponsive, including its status/heartbeat path. Two cleanup problems compound each other:
Agent.close_session()holds_sessions_semaphorewhile invoking app callbacks. The shell callback takes theShellManagercondition. Concurrent shell open/close takes that condition first, then takes_sessions_semaphorethroughSession.inc_activities_value()/dec_activities_value(): an inverse lock order.LinuxMac.terminate()can callwaitpid(pid, 0)while the manager condition is held. If collection stalls, session teardown waits for the manager while retaining the global sessions lock. Closing the Python streams also leaves the PTY master open because they useclosefd=False.This is more than a suspected explanation from logs. On a live macOS agent, an immediate shell open/close reproduced a WebSocket reader blocked in
LinuxMac.terminate()/ nativewait4, a session-close task waiting for the manager condition, and activity/status readers waiting for the global sessions lock. The process remained alive but stopped making status progress. This reproduction establishes the failure mechanism; it does not prove the precise trigger of the earlier incident that prompted the investigation.Changes
sleepsurviving shell teardown without this step.WNOHANGpolling: up to 0.5 s after TERM and 0.5 s after KILL. Retain unreaped children in one lazy, nonblocking reaper shared across shell app reloads. It polls once per second only while children are pending, sends no signals, and exits when empty. No periodic worker runs when idle.These grace periods bound deliberate polling waits, not arbitrary OS scheduling latency or every possible blocking native operation.
Regression tests included
See
tests/README.mdfor setup, limitations and negative controls. The standard-library harness compiles the actual method/class bodies from this checkout, replacing service/bootstrap/network/native dependencies. It does not read installed configuration or require a DWService account.Additional validation performed outside the repository suite
The following are completed integration experiments, not additional tests invoked by the command above:
The live relay tests originated on the same Mac that hosted the remote agent. They do not establish results for a separate originating host. Account/relay bootstrap was substituted only in the isolated network harness.
Scope
POSIX process cleanup was validated on macOS; Linux runtime and Windows terminal cleanup were not exercised. The Windows terminal implementation is unchanged; the shared Agent session-lock changes apply across platforms. Cleanup targets the owned shell and its current foreground process group, not arbitrary daemonized/background descendants. No watchdog, machine reboot policy, installer, credentials or copied deployment configuration is included.