mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
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
This commit is contained in:
1 parent
557bd9d815
commit
808d607eac
2 files changed
+102
-2
No files matched your search
@@ -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)
|
||||
}
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user