Repository navigation
fix(manager): a login ends only once its service started or its dir is gone - #20
Conversation
…s gone Review of #18 found two windows where a retry could lose its files: - A login whose child had already logged in ended (exited) before the account's service started, which ran on its own goroutine; an abort then returned at once and a retry wiped the starting account's dir. The service now starts within the login, an abort waits for it (up to the service's ready timeout), and a replacing login finds the account logged in instead. - A failed login released the account before removing its dir, so a retry could reserve it and write there first. The dir now goes under the registry's lock while the failed login still holds the account. A reaped login child is forgotten, so an abort never signals a reused pid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: LYJW131 <153256373+LYJW131@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // abortWait bounds how long an abort waits for the login to end: the killed | ||
| // child reaped and its half-made instance dir removed, or, for a child that | ||
| // had already logged in, the account's service started. | ||
| const abortWait = liteReadyTimeout + 15*time.Second |
There was a problem hiding this comment.
🔴 Login timeout response misses write deadline
If service startup is pending at 90 seconds, abortLogin can wait another 75 seconds. The login response then exceeds handleLogin's 105-second write deadline, so the client receives no timeout answer.
Learn more
A login request has a 90-second timer, and its HTTP handler gives the response a 105-second write deadline handleLogin. After the timer fires, waitForLoginFinish calls abortLogin synchronously. If the login child exited with tokens but the service is still starting, st.cmd is nil and abortLogin waits for service readiness rather than stopping it. That wait can extend past the HTTP deadline even though the service is operating within its own 60-second readiness window.
Example: The child finishes at 85 seconds, and the service becomes ready 30 seconds later. The login timer fires at 90 seconds; the handler cannot write its timeout response at 115 seconds because its write deadline was 105 seconds.
Recommended fix: Bound the HTTP handler's remaining wait by its write deadline, or separate cancellation response timing from the service-startup wait without permitting a retry to wipe the starting instance's directory. Test a child completing near loginWait with a service taking longer than loginWriteTimeout - loginWait to become ready.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed: the timeout answers first and the abort runs after it; a retry waits for it.
| if st.isAborted() && !hasTokenFiles(id) { | ||
| loginFailed(id, st, errLoginCanceled) | ||
| return | ||
| } |
There was a problem hiding this comment.
🔍 Cancellation can leave an account running
If tokens exist when a request is canceled, the service can still start after the request reports cancellation. Callers that retry can receive already login rather than a fresh login; clarify this response contract.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Documented in the README: a cancel after the child logged in does not undo the login, and a retry answers already login.
The abort can wait for a service that is starting, past the request's write deadline; it now runs after the answer, and a retry waits for it. The README says that a cancel after the child logged in does not undo the login. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: LYJW131 <153256373+LYJW131@users.noreply.github.com>
…their service started Its manager starts a logged-in account's service within the login, so a canceled login's retry cannot wipe the starting account's dir, removes a failed login's dir before it frees the account, and answers a timed-out login before its abort ends (AMDL-Web/wrapper#20). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: LYJW131 <153256373+LYJW131@users.noreply.github.com>
Follows the Devin review of #18.
st.exitedcovers it), an abort waits for it, and a replacing login answersalready loginwhen the old one got through.POST /shutdownstays unauthenticated on purpose: it exists only in the QEMU guest, whose port amdl forwards to 127.0.0.1, where/loginand/logoutare just as open.Tests:
go test -race ./....🤖 Generated with Claude Code