fix: stop shipping a stale silkd, and unbreak the Python stdio paths - #84
Closed
CMGS wants to merge 2 commits into
Closed
fix: stop shipping a stale silkd, and unbreak the Python stdio paths#84CMGS wants to merge 2 commits into
CMGS wants to merge 2 commits into
Conversation
The flavor build never passed SILKD_IMAGE, so android baked in whatever the Dockerfile ARG defaulted to (silkd 0.1.0) on every rebuild, including rebuilds triggered by a silkd change - the chain's whole purpose. It now resolves the version the same way the base build does. silkd's own image built without --locked, so a Cargo.toml bump could ship dependencies no one reviewed. sandbox-init had no PR gate at all: its suite ran only after a push to main. Python run() fed stdin to completion before reading a byte of output, so any payload past the socket buffers deadlocked against a guest that had stopped draining stdin - it now pumps stdin on its own thread, pinned by a test that deadlocks on the old code. proxy_port leaked the relay connection on every proxied stream, and kill(pid, 0) sent signal 0 (a POSIX existence probe, a no-op) where Go sends SIGKILL.
Contributor
Author
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
1 participant
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.
Stacked on #81. Five defects found by a whole-repo scan; none are introduced by that PR.
Ship-path
build-flavorsnever passedSILKD_IMAGE. Onlybuild-baseresolved it, soos-image/android/15/Dockerfile'sARG SILKD_IMAGE=…/silkd:0.1.0default won every flavor rebuild — including rebuilds triggered because silkd changed. android has been shipping silkd 0.1.0 while the crate is at 0.1.5. The job now reads the version exactly asbuild-basedoes and honours the sha-pinnedinputs.silkd_imageon a changed-carrier run.silkd/Dockerfilebuilt without--locked(boot/inithas it): aCargo.tomlbump without a matchingcargo updatewould resolve and ship un-reviewed dependency versions into the guest binary.boot/inithad no PR gate. Its fmt/clippy/test only ran viaimages.ymlon a push tomain, so a PR could not fail onsandbox-init— PID 1 in every guest. Newboot-init.ymlmirrorssilkd.yml.Python SDK
run()deadlocked on large stdin. It sent every stdin chunk andstdin_closebefore reading any output. A guest that writes while reading blocks once its stdout buffer fills, stops draining stdin, and the SDK blocks forever insendall— with no socket timeout. Go pumps stdin concurrently (go pumpStdin); Python now does the same. The regression test drives a conn whosesendblocks until output is read: it fails withstdin send deadlockedagainst the old implementation.proxy_portleaked a relay connection per stream. The happy path closed only the local socket, so every proxied TCP connection leaked the guest side until release.guest.close()moves to afinally. The OpenAI adapter inherited this leak.kill(pid, 0)meant the opposite of Go's.encode_requestdrops onlyNone, so an explicit0went on the wire and silkd'sunwrap_or(SIGKILL)kept it — signal 0 is a POSIX existence probe, i.e. a no-op. Go mapssig == 0to unset → SIGKILL.Validation
ruff checkclean;pytest152 passed (151 + the new deadlock test)run()fix makes the new test fail as designedmake go-lint0 issues,go test -racegreen across modules (unchanged by this PR)