From 808d607eac166c7d1b75a0a8da95fea65e126d3a Mon Sep 17 00:00:00 2001 From: PJ Date: Wed, 12 Aug 2026 21:38:09 +0530 Subject: [PATCH] fix(sidecarassets): publish the extracted jar through a rename Extract wrote a 96 MB jar with a plain WriteFile into a temp path every sanderling process on the host shares. On a cold host several concurrent workers all miss the checksum and all write the same path, and O_TRUNC lets one spawn a JVM against another's half-written archive. A fresh experiment host is exactly a cold host. Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX --- internal/sidecarassets/embed.go | 28 ++++++- .../sidecarassets/embed_withsidecar_test.go | 76 +++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/internal/sidecarassets/embed.go b/internal/sidecarassets/embed.go index c61976a..c48c17a 100644 --- a/internal/sidecarassets/embed.go +++ b/internal/sidecarassets/embed.go @@ -39,11 +39,35 @@ func Extract(dir string) (string, error) { } } - if err := os.WriteFile(jarPath, embeddedJAR, 0o644); err != nil { + if err := writeAtomic(jarPath, embeddedJAR); err != nil { return "", fmt.Errorf("write %s: %w", jarPath, err) } - if err := os.WriteFile(checksumPath, []byte(checksum), 0o644); err != nil { + if err := writeAtomic(checksumPath, []byte(checksum)); err != nil { return "", fmt.Errorf("write checksum: %w", err) } return jarPath, nil } + +// writeAtomic publishes content at path through a rename, so a reader never +// observes a partial file. dir is shared between every sanderling process on +// the host, so a campaign running one worker per device has several of them +// extracting the same JAR at once on a cold host; a plain write let one +// process spawn a JVM against another's half-written file. +func writeAtomic(path string, content []byte) error { + temporary, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".*.partial") + if err != nil { + return err + } + defer os.Remove(temporary.Name()) + if _, err := temporary.Write(content); err != nil { + temporary.Close() + return err + } + if err := temporary.Close(); err != nil { + return err + } + if err := os.Chmod(temporary.Name(), 0o644); err != nil { + return err + } + return os.Rename(temporary.Name(), path) +} diff --git a/internal/sidecarassets/embed_withsidecar_test.go b/internal/sidecarassets/embed_withsidecar_test.go index 3ae7400..369fe9b 100644 --- a/internal/sidecarassets/embed_withsidecar_test.go +++ b/internal/sidecarassets/embed_withsidecar_test.go @@ -6,8 +6,11 @@ import ( "bytes" "crypto/sha256" "encoding/hex" + "fmt" "os" "path/filepath" + "strings" + "sync" "testing" ) @@ -89,3 +92,76 @@ func TestExtract_RewritesIfChecksumMissing(t *testing.T) { t.Errorf("checksum should have been written: %v", err) } } + +// TestExtract_ReadersNeverSeeAPartialJAR covers the cold-host case a parallel +// campaign hits: every worker process extracts into the same shared directory +// under os.TempDir, and one of them spawns a JVM against the JAR while another +// is still writing it. A plain write truncates the file in place, so the +// reader gets a short JAR and the JVM dies on a corrupt archive. +func TestExtract_ReadersNeverSeeAPartialJAR(t *testing.T) { + directory := t.TempDir() + want := EmbeddedSHA256() + jarPath, err := Extract(directory) + if err != nil { + t.Fatal(err) + } + checksumPath := jarPath + ".sha256" + + stop := make(chan struct{}) + torn := make(chan string, 1) + var readers sync.WaitGroup + for range 4 { + readers.Add(1) + go func() { + defer readers.Done() + for { + select { + case <-stop: + return + default: + } + content, err := os.ReadFile(jarPath) + if err != nil { + continue + } + sum := sha256.Sum256(content) + if got := hex.EncodeToString(sum[:]); got != want { + select { + case torn <- fmt.Sprintf("read %d bytes, sha256 %s", len(content), got): + default: + } + return + } + } + }() + } + + for range 8 { + // Dropping the checksum is what a cold host looks like: no extraction + // has been recorded, so every caller writes the JAR again. + if err := os.Remove(checksumPath); err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + if _, err := Extract(directory); err != nil { + t.Fatal(err) + } + } + close(stop) + readers.Wait() + + select { + case detail := <-torn: + t.Fatalf("a reader observed a partial JAR while another caller extracted: %s", detail) + default: + } + + entries, err := os.ReadDir(directory) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.Contains(entry.Name(), ".partial") { + t.Errorf("Extract left a staging file behind: %s", entry.Name()) + } + } +}