Repository navigation
fix(manager): the guest keeps its writes when stopped, and a login can be canceled - #18
Conversation
…n be canceled A host that stopped the QEMU guest killed it, and ext4's delayed allocation kept ~30 s of written data in guest memory: an installed payload, or an instances.json and the tokens of a fresh login, were lost with it. - init mounts /data with commit=1,nodelalloc, falls back to the defaults, formats only a disk with no ext4 magic, and syncs and unmounts before powering off. - POST /shutdown, served only in the guest (init sets WRAPPER_MANAGER_GUEST=1), syncs and stops the manager, so the host can power the guest off instead of killing it. - instances.json and .payload-source are written with fsync and a synced rename; the payload's files and a login's tokens are synced before they are counted on. - With -payload-sha256 the digest alone names the installed payload, so a host may serve the same zip from a new URL each boot without a reinstall. A login honours the request's context: when the client goes away, or the 90 s login wait passes, its child is killed and its instance dir removed before the answer, and POST /login gets a write deadline past that wait instead of the server's 60 s. A new login of an account replaces one still in flight, and a replaced login's late failure no longer removes the new login's dir. 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}
| select { | ||
| case <-st.exited: | ||
| case <-time.After(abortWait): | ||
| log.Warnf("[wrapper %s] the canceled login's child did not exit within %s", shortID(st.Id), abortWait) |
There was a problem hiding this comment.
🔴 Canceled login still starts its service
If the login child exits before cancellation, abortLogin returns while service startup continues. st.exited tracks only the login child, so the canceled account can still start and persist.
Learn more
A successful login has two phases: runLoginChild waits for the one-shot login child, then finishLoginSuccess starts a long-running service in a separate goroutine. st.exited closes when the first phase returns, before service startup completes. Canceling during the second phase therefore returns immediately without stopping that service. The service can persist the account after the caller sees a canceled login, and a retry can erase its token directory.
Example: The login child writes tokens and exits after two seconds. The service takes ten seconds to report ready. The client disconnects at second three; abortLogin returns, but the service later becomes ready and saves the account.
Recommended fix: Track and cancel the whole login lifecycle, including the goroutine in finishLoginSuccess. Do not resolve cancellation or start another attempt until any service process is stopped, reaped, and its directory cleanup is complete.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if releaseLogin(id, st) { | ||
| _ = RemoveWrapperDataQuiet(id) | ||
| } |
There was a problem hiding this comment.
🔴 Failed login cleanup erases retry data
When a retry starts after loginFailed releases its registry entry, the old cleanup can erase the retry's directory. reserveLogin admits the retry before RemoveWrapperDataQuiet completes, so its tokens can disappear.
Learn more
The login registry serializes attempts for the same account, and both attempts use the same directory. Here releaseLogin frees that registry slot before RemoveWrapperDataQuiet removes the directory. A concurrent request can reserve the freed slot and start a child in that directory before removal finishes. The old cleanup then deletes files owned by the new login.
Example: An old login exits unsuccessfully and frees its slot. A retry creates STOREFRONT_ID in the same directory while the old RemoveAll runs. The retry loses its token file and cannot complete successfully.
Recommended fix: Keep the old login reserved until its child is reaped and directory cleanup finishes. Release the registry entry atomically with making the account available to a new attempt, while retaining pointer checks to protect replacements.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if os.Getenv(guestEnv) == "1" { | ||
| mux.HandleFunc("/shutdown", handleShutdown(sigCh)) | ||
| } |
There was a problem hiding this comment.
🟥 Unauthenticated shutdown powers off the guest
Any client reaching the forwarded guest port can call POST /shutdown without credentials. The request stops the manager, and guest init powers off the VM.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Kept as is: /shutdown exists only in the QEMU guest (init sets WRAPPER_MANAGER_GUEST), whose port amdl forwards to 127.0.0.1 only, where /login and /logout are just as open. A container manager never serves it.
amdl stopped the QEMU guest by killing it, and ext4's delayed allocation kept about 30 s of writes in guest memory, so a payload installed just before a stop was downloaded again and a fresh login's
instances.jsonand tokens could be lost.guest/init:/datamounts withcommit=1,nodelalloc(falls back to the defaults); a disk that holds ext4 but does not mount is never formatted; sync and unmount beforepoweroff -f.POST /shutdown, only in the guest (WRAPPER_MANAGER_GUEST=1from init): sync, answer, stop as on SIGTERM. amdl calls it before it falls back to killing QEMU.instances.jsonand.payload-sourceare written with fsync and a synced rename; the payload and a login's tokens are synced before they count.-payload-sha256the digest alone names the installed payload: amdl now serves the zip to the guest from the host on a new port each boot./loginfollows the request context. A client that goes away, or the 90 s login wait, kills the login child and removes its instance dir before the answer;/logingets a 105 s write deadline instead of the server's 60 s. A new login of an account replaces one still in flight (e.g. waiting for a code), and a replaced login's late failure no longer removes the new login's dir.Tests:
go test ./...and-raceon the login and shutdown tests.🤖 Generated with Claude Code