on main help sign in
Merge pull request #2876 from charmbracelet/client-server-race

fix: client-server startup race

commit61fa910250f841f8648faef295de40f2899fc9cb authorChristian Rocha <christian@rocha.is> dateThu 14 May 2026, 09:56 (5 months ago) parent3625adeeb03b3b
6 files changed+656−53
internal/client/client.go+8 internal/cmd/clientserverrace/race_test.go+361 internal/cmd/root.go+221−47 internal/cmd/server.go+6−6 internal/cmd/spawnlock_other.go+28 internal/cmd/spawnlock_windows.go+32
internal/client/client.go+8
@@ -121,6 +121,14 @@ func (c *Client) ShutdownServer(ctx context.Context) error {
121121 return nil
122122}
123123
124+// Dial opens a connection to the server using the same scheme-aware
125+// logic the client uses for its HTTP transport. Exposed so callers can
126+// reuse the dialer when they need to construct sibling HTTP transports
127+// (e.g. a readiness probe in the CLI).
128+func (c *Client) Dial(ctx context.Context, network, address string) (net.Conn, error) {
129+ return c.dialer(ctx, network, address)
130+}
131+
124132func (c *Client) dialer(ctx context.Context, network, address string) (net.Conn, error) {
125133 d := net.Dialer{
126134 Timeout: 30 * time.Second,
internal/cmd/clientserverrace/race_test.gonew file+361
@@ -0,0 +1,361 @@
1+// Package clientserverrace_test is a regression test for the
2+// CRUSH_CLIENT_SERVER=1 socket-init race documented in
3+// docs/notes/2026-05-11-client-server-socket-init-race.md (item F5).
4+//
5+// It lives in its own directory so it can build even if other test
6+// files in internal/cmd are temporarily broken — this test only needs
7+// the binary, not the cmd package.
8+package clientserverrace_test
9+
10+import (
11+ "context"
12+ "fmt"
13+ "net"
14+ "net/http"
15+ "os"
16+ "os/exec"
17+ "path/filepath"
18+ "runtime"
19+ "strings"
20+ "sync"
21+ "sync/atomic"
22+ "testing"
23+ "time"
24+)
25+
26+// readinessErrSubstr is the user-visible error string emitted by
27+// ensureServer when it gives up waiting for the server socket /
28+// readiness probe (internal/cmd/root.go). Seeing this in any client's
29+// output means the race fired.
30+const readinessErrSubstr = "failed to initialize crush server"
31+
32+// numClients is intentionally larger than the typical CPU count to
33+// ensure the spawn lock + readiness probe are exercised under
34+// contention.
35+const numClients = 8
36+
37+// clientTimeout bounds each child invocation. It only needs to be long
38+// enough for the spawn-and-readiness phase to complete on a cold cache;
39+// after that, the client may legitimately keep running (e.g.
40+// subscribing to server events) and we'll cancel it. The race we care
41+// about is observable strictly within ensureServer.
42+const clientTimeout = 15 * time.Second
43+
44+func TestClientServerSpawnRace(t *testing.T) {
45+ if testing.Short() {
46+ t.Skip("skipping client/server spawn race test in -short mode")
47+ }
48+ // The race and its fix are unix-socket specific. Windows uses
49+ // named pipes via a different code path; not covered here.
50+ if runtime.GOOS == "windows" {
51+ t.Skip("skipping unix-socket specific race test on windows")
52+ }
53+ if _, err := exec.LookPath("go"); err != nil {
54+ t.Skip("skipping: 'go' not available on PATH")
55+ }
56+
57+ repoRoot := repoRootFromTest(t)
58+ bin := buildCrushBinary(t, repoRoot)
59+
60+ // Use /tmp directly so the unix socket path stays under the
61+ // 104-char sockaddr_un limit on darwin. t.TempDir() can return a
62+ // path inside /var/folders/... that is too long.
63+ runDir, err := os.MkdirTemp("/tmp", "crush-race-")
64+ if err != nil {
65+ t.Fatalf("mkdtemp: %v", err)
66+ }
67+ t.Cleanup(func() { _ = os.RemoveAll(runDir) })
68+
69+ socketPath := filepath.Join(runDir, "crush.sock")
70+ host := "unix://" + socketPath
71+
72+ // Fresh, isolated XDG/HOME so we don't touch the user's real
73+ // state or any other test's cache. These all live under runDir
74+ // so cleanup is one RemoveAll.
75+ cacheHome := filepath.Join(runDir, "cache")
76+ dataHome := filepath.Join(runDir, "data")
77+ configHome := filepath.Join(runDir, "config")
78+ homeDir := filepath.Join(runDir, "home")
79+ for _, d := range []string{cacheHome, dataHome, configHome, homeDir} {
80+ if err := os.MkdirAll(d, 0o700); err != nil {
81+ t.Fatalf("mkdir %s: %v", d, err)
82+ }
83+ }
84+
85+ env := append(
86+ os.Environ(),
87+ "CRUSH_CLIENT_SERVER=1",
88+ "XDG_CACHE_HOME="+cacheHome,
89+ "XDG_DATA_HOME="+dataHome,
90+ "XDG_CONFIG_HOME="+configHome,
91+ "HOME="+homeDir,
92+ // Belt-and-suspenders: if anything tries to talk to a real
93+ // provider, fail loudly rather than make a network call.
94+ "CRUSH_DISABLE_PROVIDER_AUTO_UPDATE=1",
95+ )
96+
97+ // Make sure no server is up before we start.
98+ if _, err := os.Stat(socketPath); err == nil {
99+ t.Fatalf("socket %s exists before test started", socketPath)
100+ }
101+
102+ // Always try to shut down any server we spawned, regardless of
103+ // outcome.
104+ t.Cleanup(func() { shutdownServer(t, socketPath) })
105+
106+ type result struct {
107+ idx int
108+ stdout string
109+ stderr string
110+ }
111+ results := make(chan result, numClients)
112+
113+ // Probe /v1/health concurrently while the clients are still
114+ // running. The server self-shuts-down when the last workspace is
115+ // released (internal/backend/backend.go:DeleteWorkspace), so once
116+ // all clients exit cleanly the socket may legitimately be gone —
117+ // asserting the socket post-hoc would race with that documented
118+ // self-shutdown. Instead we require that during the parallel run
119+ // at least one /v1/health probe got a 2xx, which proves the
120+ // spawn-and-readiness path actually produced a live server.
121+ var sawHealthy atomic.Bool
122+ probeDone := make(chan struct{})
123+ stopProbe := make(chan struct{})
124+
125+ var wg sync.WaitGroup
126+ start := make(chan struct{})
127+
128+ go func() {
129+ defer close(probeDone)
130+ <-start
131+ deadline := time.Now().Add(clientTimeout)
132+ for time.Now().Before(deadline) {
133+ select {
134+ case <-stopProbe:
135+ return
136+ default:
137+ }
138+ if err := pingHealth(socketPath); err == nil {
139+ sawHealthy.Store(true)
140+ return
141+ }
142+ select {
143+ case <-stopProbe:
144+ return
145+ case <-time.After(50 * time.Millisecond):
146+ }
147+ }
148+ }()
149+
150+ for i := range numClients {
151+ wg.Add(1)
152+ go func(i int) {
153+ defer wg.Done()
154+
155+ // Each client gets its own working directory so the
156+ // per-client workspace registration paths don't collide
157+ // in confusing ways.
158+ cwd := filepath.Join(runDir, fmt.Sprintf("ws-%d", i))
159+ if err := os.MkdirAll(cwd, 0o700); err != nil {
160+ results <- result{idx: i, stderr: fmt.Sprintf("mkdir cwd: %v", err)}
161+ return
162+ }
163+
164+ ctx, cancel := context.WithTimeout(context.Background(), clientTimeout)
165+ defer cancel()
166+
167+ // `crush run` exercises connectToServer (which is where
168+ // the readiness race lives). On a fresh sandbox the
169+ // command may legitimately keep running past the race
170+ // (e.g. waiting on event subscriptions); the context
171+ // timeout above bounds that. We assert race outcomes
172+ // purely from output, not exit code.
173+ c := exec.CommandContext(
174+ ctx, bin,
175+ "--host", host,
176+ "--cwd", cwd,
177+ "run", "hi",
178+ )
179+ c.Env = env
180+ var outBuf, errBuf strings.Builder
181+ c.Stdout = &outBuf
182+ c.Stderr = &errBuf
183+
184+ <-start
185+ _ = c.Run()
186+ results <- result{
187+ idx: i,
188+ stdout: outBuf.String(),
189+ stderr: errBuf.String(),
190+ }
191+ }(i)
192+ }
193+
194+ close(start) // release all clients as simultaneously as possible
195+ wg.Wait()
196+ close(results)
197+ close(stopProbe)
198+ <-probeDone
199+
200+ var raceFailures []string
201+ for r := range results {
202+ if strings.Contains(r.stderr, readinessErrSubstr) ||
203+ strings.Contains(r.stdout, readinessErrSubstr) {
204+ raceFailures = append(raceFailures, fmt.Sprintf(
205+ "client %d: readiness error in output\nstderr:\n%s\nstdout:\n%s",
206+ r.idx, r.stderr, r.stdout,
207+ ))
208+ }
209+ }
210+
211+ if len(raceFailures) > 0 {
212+ t.Fatalf(
213+ "client/server spawn race regressed: %d/%d clients failed\n\n%s",
214+ len(raceFailures), numClients,
215+ strings.Join(raceFailures, "\n---\n"),
216+ )
217+ }
218+
219+ // Positive sanity check: at some point during the parallel run a
220+ // /v1/health probe must have succeeded. We deliberately do *not*
221+ // stat the socket post-hoc: when every client returns cleanly
222+ // (e.g. exits early because no providers are configured), the
223+ // last DeleteWorkspace triggers the server's self-shutdown and
224+ // the socket disappears. That is correct behaviour, not a race
225+ // regression.
226+ if !sawHealthy.Load() {
227+ t.Fatalf("no /v1/health probe succeeded on %s while %d clients were running",
228+ socketPath, numClients)
229+ }
230+}
231+
232+// pingHealth issues a single GET /v1/health over the unix socket and
233+// requires a 2xx response.
234+func pingHealth(socketPath string) error {
235+ tr := &http.Transport{
236+ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
237+ var d net.Dialer
238+ return d.DialContext(ctx, "unix", socketPath)
239+ },
240+ }
241+ defer tr.CloseIdleConnections()
242+ hc := &http.Client{Transport: tr, Timeout: 2 * time.Second}
243+
244+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
245+ defer cancel()
246+ req, err := http.NewRequestWithContext(ctx, http.MethodGet,
247+ "http://crush.local/v1/health", nil)
248+ if err != nil {
249+ return err
250+ }
251+ rsp, err := hc.Do(req)
252+ if err != nil {
253+ return err
254+ }
255+ defer rsp.Body.Close()
256+ if rsp.StatusCode < 200 || rsp.StatusCode >= 300 {
257+ return fmt.Errorf("health check returned %s", rsp.Status)
258+ }
259+ return nil
260+}
261+
262+// repoRootFromTest walks up from this test file's directory to find
263+// the repo root (the directory containing go.mod). Walking up by a
264+// fixed count is fragile across reorganisations.
265+func repoRootFromTest(t *testing.T) string {
266+ t.Helper()
267+ cwd, err := os.Getwd()
268+ if err != nil {
269+ t.Fatalf("getwd: %v", err)
270+ }
271+ dir := cwd
272+ for {
273+ if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
274+ return dir
275+ }
276+ parent := filepath.Dir(dir)
277+ if parent == dir {
278+ t.Fatalf("could not find go.mod walking up from %s", cwd)
279+ }
280+ dir = parent
281+ }
282+}
283+
284+// buildCrushBinary builds the crush binary once at the start of the
285+// test and returns the absolute path. Subsequent t.Cleanup removes
286+// the built artefact.
287+func buildCrushBinary(t *testing.T, repoRoot string) string {
288+ t.Helper()
289+
290+ binDir, err := os.MkdirTemp("", "crush-race-bin-")
291+ if err != nil {
292+ t.Fatalf("mkdtemp bin: %v", err)
293+ }
294+ t.Cleanup(func() { _ = os.RemoveAll(binDir) })
295+
296+ binPath := filepath.Join(binDir, "crush")
297+
298+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
299+ defer cancel()
300+
301+ cmd := exec.CommandContext(ctx, "go", "build", "-o", binPath, ".")
302+ cmd.Dir = repoRoot
303+ // Match the project's standard build flags. CGO_ENABLED=0 keeps
304+ // the binary statically linked and avoids surprising the test on
305+ // hosts without a C toolchain.
306+ cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
307+ out, err := cmd.CombinedOutput()
308+ if err != nil {
309+ t.Fatalf("go build crush: %v\n%s", err, out)
310+ }
311+ return binPath
312+}
313+
314+// shutdownServer best-effort terminates any crush server bound to
315+// socketPath by POSTing to /v1/control. We don't import the project's
316+// own client package to keep this test free of internal API churn.
317+func shutdownServer(t *testing.T, socketPath string) {
318+ t.Helper()
319+ if _, err := os.Stat(socketPath); err != nil {
320+ return
321+ }
322+
323+ tr := &http.Transport{
324+ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
325+ var d net.Dialer
326+ return d.DialContext(ctx, "unix", socketPath)
327+ },
328+ }
329+ hc := &http.Client{Transport: tr, Timeout: 5 * time.Second}
330+ defer tr.CloseIdleConnections()
331+
332+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
333+ defer cancel()
334+
335+ body := strings.NewReader(`{"command":"shutdown"}`)
336+ req, err := http.NewRequestWithContext(ctx, http.MethodPost,
337+ "http://crush.local/v1/control", body)
338+ if err != nil {
339+ t.Logf("shutdown: build request: %v", err)
340+ return
341+ }
342+ req.Header.Set("Content-Type", "application/json")
343+
344+ resp, err := hc.Do(req)
345+ if err != nil {
346+ // Server may already be gone — not an error.
347+ t.Logf("shutdown: %v (probably already exited)", err)
348+ return
349+ }
350+ _ = resp.Body.Close()
351+
352+ // Wait briefly for the socket to disappear so the next test
353+ // using the same path doesn't race.
354+ deadline := time.Now().Add(5 * time.Second)
355+ for time.Now().Before(deadline) {
356+ if _, err := os.Stat(socketPath); err != nil {
357+ return
358+ }
359+ time.Sleep(50 * time.Millisecond)
360+ }
361+}
internal/cmd/root.go+221−47
@@ -9,6 +9,8 @@ import (
99 "io"
1010 "io/fs"
1111 "log/slog"
12+ "net"
13+ "net/http"
1214 "net/url"
1315 "os"
1416 "os/exec"
@@ -357,22 +359,7 @@ func connectToServer(cmd *cobra.Command) (*client.Client, *proto.Workspace, func
357359
358360 ws, err := c.CreateWorkspace(ctx, wsReq)
359361 if err != nil {
360− // The server socket may exist before the HTTP handler is ready.
361− // Retry a few times with a short backoff.
362− for range 5 {
363− select {
364− case <-ctx.Done():
365− return nil, nil, nil, ctx.Err()
366− case <-time.After(200 * time.Millisecond):
367− }
368− ws, err = c.CreateWorkspace(ctx, wsReq)
369− if err == nil {
370− break
371− }
372− }
373− if err != nil {
374− return nil, nil, nil, fmt.Errorf("failed to create workspace: %v", err)
375− }
362+ return nil, nil, nil, fmt.Errorf("failed to create workspace: %v", err)
376363 }
377364
378365 if shouldEnableMetrics(ws.Config) {
@@ -396,55 +383,239 @@ func ensureServer(cmd *cobra.Command, hostURL *url.URL) error {
396383 switch hostURL.Scheme {
397384 case "unix", "npipe":
398385 needsStart := false
399− if _, err := os.Stat(hostURL.Host); err != nil && errors.Is(err, fs.ErrNotExist) {
386+ _, statErr := os.Stat(hostURL.Host)
387+ switch {
388+ case statErr == nil:
389+ restarted, err := restartIfStale(cmd, hostURL)
390+ if err != nil {
391+ slog.Warn("Failed to check server version", "error", err)
392+ }
393+ needsStart = restarted || err != nil
394+ case errors.Is(statErr, fs.ErrNotExist):
400395 needsStart = true
401− } else if err == nil {
402− if err := restartIfStale(cmd, hostURL); err != nil {
403− slog.Warn("Failed to check server version, restarting", "error", err)
404− needsStart = true
396+ default:
397+ slog.Warn("Unexpected error stat'ing server socket, attempting cleanup",
398+ "path", hostURL.Host, "error", statErr)
399+ if err := os.Remove(hostURL.Host); err != nil && !errors.Is(err, fs.ErrNotExist) {
400+ return fmt.Errorf("failed to remove stale server socket %q: %v", hostURL.Host, err)
405401 }
402+ needsStart = true
406403 }
407404
408405 if needsStart {
409− if err := startDetachedServer(cmd); err != nil {
410− return err
406+ if err := spawnAndWaitReady(cmd, hostURL); err != nil {
407+ return fmt.Errorf("failed to initialize crush server: %v", err)
411408 }
409+ return nil
412410 }
413411
414− var err error
415− for range 10 {
416− _, err = os.Stat(hostURL.Host)
417− if err == nil {
418− break
419− }
420− select {
421− case <-cmd.Context().Done():
422− return cmd.Context().Err()
423− case <-time.After(100 * time.Millisecond):
412+ if err := waitForServerReady(cmd.Context(), hostURL); err != nil {
413+ return fmt.Errorf("failed to initialize crush server: %v", err)
414+ }
415+ }
416+
417+ return nil
418+}
419+
420+// spawnAndWaitReady serializes the spawn-and-wait-for-readiness sequence
421+// across concurrent clients via an exclusive flock on
422+// $XDG_CACHE_HOME/crush/server-<safeHost>/start.lock.
423+//
424+// After acquiring the lock it re-probes readiness so that a client that
425+// blocked while another client was spawning can skip its own spawn and
426+// just use the now-running server. The lock is held only for the
427+// duration of "spawn + readiness probe" and released before the caller
428+// resumes its normal lifetime.
429+func spawnAndWaitReady(cmd *cobra.Command, hostURL *url.URL) error {
430+ chDir, err := perHostServerDir(hostURL)
431+ if err != nil {
432+ return err
433+ }
434+ release, err := acquireSpawnLock(filepath.Join(chDir, "start.lock"))
435+ if err != nil {
436+ // If the lock itself is unavailable, fall back to the
437+ // unsynchronized path rather than blocking the user.
438+ slog.Warn("Failed to acquire spawn lock, proceeding without single-flight", "error", err)
439+ if err := startDetachedServer(cmd, hostURL); err != nil {
440+ return err
441+ }
442+ return waitForServerReady(cmd.Context(), hostURL)
443+ }
444+ defer release()
445+
446+ // Another client may have just finished spawning while we were
447+ // waiting on the lock; if the server is already responsive, skip
448+ // the spawn entirely.
449+ probeCtx, cancel := context.WithTimeout(cmd.Context(), 200*time.Millisecond)
450+ probeErr := quickHealthProbe(probeCtx, hostURL)
451+ cancel()
452+ if probeErr == nil {
453+ return nil
454+ }
455+
456+ if err := startDetachedServer(cmd, hostURL); err != nil {
457+ return err
458+ }
459+ return waitForServerReady(cmd.Context(), hostURL)
460+}
461+
462+// quickHealthProbe issues a single readiness request with the caller's
463+// context and returns nil iff the server is responsive right now.
464+func quickHealthProbe(ctx context.Context, hostURL *url.URL) error {
465+ httpClient, reqURL, err := readinessHTTPClient(hostURL)
466+ if err != nil {
467+ return err
468+ }
469+ return probeHealth(ctx, httpClient, reqURL, hostURL)
470+}
471+
472+// perHostServerDir returns (and creates) the cache directory used for
473+// per-host server state (logs, start.lock, etc.). The path is derived
474+// from the parsed host URL rather than the global flag so the same key
475+// is computed regardless of where the host came from.
476+func perHostServerDir(hostURL *url.URL) (string, error) {
477+ chDir := filepath.Join(config.GlobalCacheDir(), "server-"+safeHostName(hostURL))
478+ if err := os.MkdirAll(chDir, 0o700); err != nil {
479+ return "", fmt.Errorf("failed to create server working directory: %v", err)
480+ }
481+ return chDir, nil
482+}
483+
484+// safeHostName returns a filesystem-safe identifier for hostURL,
485+// suitable for use as a directory name. It mirrors the input shape of
486+// the --host flag so client and server compute the same key.
487+func safeHostName(hostURL *url.URL) string {
488+ return safeNameRegexp.ReplaceAllString(
489+ hostURL.Scheme+"://"+hostURL.Host+hostURL.Path, "_")
490+}
491+
492+// serverReadyTimeout returns the total budget for the readiness probe.
493+// Overridable via CRUSH_SERVER_READY_TIMEOUT (parsed as a Go duration).
494+func serverReadyTimeout() time.Duration {
495+ const def = 10 * time.Second
496+ v := os.Getenv("CRUSH_SERVER_READY_TIMEOUT")
497+ if v == "" {
498+ return def
499+ }
500+ d, err := time.ParseDuration(v)
501+ if err != nil || d <= 0 {
502+ return def
503+ }
504+ return d
505+}
506+
507+// waitForServerReady polls GET /v1/health until the server responds with
508+// any 2xx status or the total timeout elapses. Each attempt uses a short
509+// per-attempt timeout so a hung listener doesn't burn the whole budget.
510+//
511+// The HTTP transport is built to mirror how *client.Client dials so the
512+// same unix socket / npipe / tcp setups all work uniformly here.
513+func waitForServerReady(ctx context.Context, hostURL *url.URL) error {
514+ httpClient, reqURL, err := readinessHTTPClient(hostURL)
515+ if err != nil {
516+ return err
517+ }
518+
519+ const perAttempt = 100 * time.Millisecond
520+ deadline := time.Now().Add(serverReadyTimeout())
521+
522+ var lastErr error
523+ for {
524+ if err := ctx.Err(); err != nil {
525+ return err
526+ }
527+ if time.Now().After(deadline) {
528+ if lastErr != nil {
529+ return lastErr
424530 }
531+ return fmt.Errorf("timed out waiting for server readiness")
425532 }
426− if err != nil {
427− return fmt.Errorf("failed to initialize crush server: %v", err)
533+
534+ attemptCtx, cancel := context.WithTimeout(ctx, perAttempt)
535+ err := probeHealth(attemptCtx, httpClient, reqURL, hostURL)
536+ cancel()
537+ if err == nil {
538+ return nil
539+ }
540+ lastErr = err
541+
542+ select {
543+ case <-ctx.Done():
544+ return ctx.Err()
545+ case <-time.After(perAttempt):
428546 }
429547 }
548+}
430549
550+// readinessHTTPClient builds an *http.Client whose transport dials the
551+// server using the same scheme-aware logic as *client.Client (unix
552+// socket, named pipe, or tcp).
553+func readinessHTTPClient(hostURL *url.URL) (*http.Client, string, error) {
554+ c, err := client.NewClient("", hostURL.Scheme, hostURL.Host)
555+ if err != nil {
556+ return nil, "", err
557+ }
558+
559+ tr := http.DefaultTransport.(*http.Transport).Clone()
560+ tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
561+ return c.Dial(ctx, network, addr)
562+ }
563+ if hostURL.Scheme == "unix" || hostURL.Scheme == "npipe" {
564+ tr.DisableCompression = true
565+ }
566+
567+ httpClient := &http.Client{Transport: tr}
568+
569+ // For unix sockets / named pipes we still need a syntactically valid
570+ // HTTP URL; the actual address is resolved by the dialer.
571+ host := hostURL.Host
572+ if hostURL.Scheme == "unix" || hostURL.Scheme == "npipe" {
573+ host = client.DummyHost
574+ }
575+ reqURL := (&url.URL{Scheme: "http", Host: host, Path: "/v1/health"}).String()
576+ return httpClient, reqURL, nil
577+}
578+
579+// probeHealth issues a single GET to the readiness endpoint and treats
580+// any 2xx response as success.
581+func probeHealth(ctx context.Context, h *http.Client, reqURL string, hostURL *url.URL) error {
582+ req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
583+ if err != nil {
584+ return err
585+ }
586+ if hostURL.Scheme == "unix" || hostURL.Scheme == "npipe" {
587+ req.Host = client.DummyHost
588+ }
589+ rsp, err := h.Do(req)
590+ if err != nil {
591+ return err
592+ }
593+ defer rsp.Body.Close()
594+ _, _ = io.Copy(io.Discard, rsp.Body)
595+ if rsp.StatusCode < 200 || rsp.StatusCode >= 300 {
596+ return fmt.Errorf("server health check failed: %s", rsp.Status)
597+ }
431598 return nil
432599}
433600
434601// restartIfStale checks whether the running server matches the current
435602// client version. When they differ, it sends a shutdown command and
436603// removes the stale socket so the caller can start a fresh server.
437−func restartIfStale(cmd *cobra.Command, hostURL *url.URL) error {
604+//
605+// It returns restarted=true when it has shut down a stale server and the
606+// caller must spawn a new one. When the server matches the client version
607+// (or the check itself fails), restarted is false.
608+func restartIfStale(cmd *cobra.Command, hostURL *url.URL) (restarted bool, err error) {
438609 c, err := client.NewClient("", hostURL.Scheme, hostURL.Host)
439610 if err != nil {
440− return err
611+ return false, err
441612 }
442613 vi, err := c.VersionInfo(cmd.Context())
443614 if err != nil {
444− return err
615+ return false, err
445616 }
446617 if vi.Version == version.Version {
447− return nil
618+ return false, nil
448619 }
449620 slog.Info("Server version mismatch, restarting",
450621 "server", vi.Version,
@@ -458,27 +629,26 @@ func restartIfStale(cmd *cobra.Command, hostURL *url.URL) error {
458629 }
459630 select {
460631 case <-cmd.Context().Done():
461− return cmd.Context().Err()
632+ return true, cmd.Context().Err()
462633 case <-time.After(100 * time.Millisecond):
463634 }
464635 }
465636 // Force-remove if the socket is still lingering.
466637 _ = os.Remove(hostURL.Host)
467− return nil
638+ return true, nil
468639}
469640
470641var safeNameRegexp = regexp.MustCompile(`[^a-zA-Z0-9._-]`)
471642
472−func startDetachedServer(cmd *cobra.Command) error {
643+func startDetachedServer(cmd *cobra.Command, hostURL *url.URL) error {
473644 exe, err := os.Executable()
474645 if err != nil {
475646 return fmt.Errorf("failed to get executable path: %v", err)
476647 }
477648
478− safeClientHost := safeNameRegexp.ReplaceAllString(clientHost, "_")
479− chDir := filepath.Join(config.GlobalCacheDir(), "server-"+safeClientHost)
480− if err := os.MkdirAll(chDir, 0o700); err != nil {
481− return fmt.Errorf("failed to create server working directory: %v", err)
649+ chDir, err := perHostServerDir(hostURL)
650+ if err != nil {
651+ return err
482652 }
483653
484654 cmdArgs := []string{"server"}
@@ -486,7 +656,11 @@ func startDetachedServer(cmd *cobra.Command) error {
486656 cmdArgs = append(cmdArgs, "--host", clientHost)
487657 }
488658
489− c := exec.CommandContext(cmd.Context(), exe, cmdArgs...)
659+ // Use context.Background() so the parent's context cancellation does not
660+ // kill the spawned server. detachProcess (Setsid on !windows,
661+ // DETACHED_PROCESS on windows) is what truly detaches the child from
662+ // this process's lifetime.
663+ c := exec.CommandContext(context.Background(), exe, cmdArgs...)
490664 stdoutPath := filepath.Join(chDir, "stdout.log")
491665 stderrPath := filepath.Join(chDir, "stderr.log")
492666 detachProcess(c)
internal/cmd/server.go+6−6
@@ -42,7 +42,12 @@ var serverCmd = &cobra.Command{
4242 return fmt.Errorf("failed to load configuration: %v", err)
4343 }
4444
45− logFile := filepath.Join(config.GlobalCacheDir(), "server-"+safeNameRegexp.ReplaceAllString(serverHost, "_"), "crush.log")
45+ hostURL, err := server.ParseHostURL(serverHost)
46+ if err != nil {
47+ return fmt.Errorf("invalid server host: %v", err)
48+ }
49+
50+ logFile := filepath.Join(config.GlobalCacheDir(), "server-"+safeHostName(hostURL), "crush.log")
4651
4752 if term.IsTerminal(os.Stderr.Fd()) {
4853 crushlog.Setup(logFile, debug, os.Stderr)
@@ -50,11 +55,6 @@ var serverCmd = &cobra.Command{
5055 crushlog.Setup(logFile, debug)
5156 }
5257
53− hostURL, err := server.ParseHostURL(serverHost)
54− if err != nil {
55− return fmt.Errorf("invalid server host: %v", err)
56− }
57−
5858 srv := server.NewServer(cfg, hostURL.Scheme, hostURL.Host)
5959 srv.SetLogger(slog.Default())
6060 slog.Info("Starting Crush server...", "addr", serverHost)
internal/cmd/spawnlock_other.gonew file+28
@@ -0,0 +1,28 @@
1+//go:build !windows
2+
3+package cmd
4+
5+import (
6+ "fmt"
7+ "os"
8+
9+ "golang.org/x/sys/unix"
10+)
11+
12+// acquireSpawnLock takes an exclusive flock on the given file (creating
13+// it if necessary) and returns a release function that unlocks and
14+// closes the file. Blocks until the lock is acquired.
15+func acquireSpawnLock(path string) (func(), error) {
16+ f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
17+ if err != nil {
18+ return nil, fmt.Errorf("open spawn lock %q: %v", path, err)
19+ }
20+ if err := unix.Flock(int(f.Fd()), unix.LOCK_EX); err != nil {
21+ _ = f.Close()
22+ return nil, fmt.Errorf("flock spawn lock %q: %v", path, err)
23+ }
24+ return func() {
25+ _ = unix.Flock(int(f.Fd()), unix.LOCK_UN)
26+ _ = f.Close()
27+ }, nil
28+}
internal/cmd/spawnlock_windows.gonew file+32
@@ -0,0 +1,32 @@
1+//go:build windows
2+
3+package cmd
4+
5+import (
6+ "fmt"
7+ "math"
8+ "os"
9+
10+ "golang.org/x/sys/windows"
11+)
12+
13+// acquireSpawnLock takes an exclusive lock on the given file (creating
14+// it if necessary) using LockFileEx, and returns a release function
15+// that unlocks and closes the file. Blocks until the lock is acquired.
16+func acquireSpawnLock(path string) (func(), error) {
17+ f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
18+ if err != nil {
19+ return nil, fmt.Errorf("open spawn lock %q: %v", path, err)
20+ }
21+ h := windows.Handle(f.Fd())
22+ ol := new(windows.Overlapped)
23+ if err := windows.LockFileEx(h, windows.LOCKFILE_EXCLUSIVE_LOCK, 0, math.MaxUint32, math.MaxUint32, ol); err != nil {
24+ _ = f.Close()
25+ return nil, fmt.Errorf("LockFileEx spawn lock %q: %v", path, err)
26+ }
27+ return func() {
28+ ol := new(windows.Overlapped)
29+ _ = windows.UnlockFileEx(windows.Handle(f.Fd()), 0, math.MaxUint32, math.MaxUint32, ol)
30+ _ = f.Close()
31+ }, nil
32+}