diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index c1c68210..7db7440d 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -64,17 +64,17 @@ func (m *Merlin) PreRun() error { if err := ntp.WaitNvram(); err != nil { return err } - // Wait until directories mounted. + // Wait until directories mounted, but never block router startup forever. for _, dir := range []string{"/tmp", "/proc"} { - waitDirExists(dir) - } - // Wait dnsmasq started. - for { - out, _ := exec.Command("pidof", "dnsmasq").CombinedOutput() - if len(bytes.TrimSpace(out)) > 0 { - break + if err := waitDirExists(dir, 60*time.Second); err != nil { + return err } - time.Sleep(time.Second) + } + // dnsmasq should be available during normal Merlin boot. A bounded wait + // keeps a broken/missing dnsmasq from wedging ctrld's one-shot startup + // indefinitely. + if err := waitProcess("dnsmasq", 60*time.Second); err != nil { + return err } return nil } @@ -255,12 +255,56 @@ func merlinParsePostConf(buf []byte) []byte { return buf } -// waitDirExists waits until the specified directory exists, polling its existence every second. -func waitDirExists(dir string) { +func waitUntil(timeout, interval time.Duration, probe func() (bool, error)) error { + deadline := time.Now().Add(timeout) for { - if _, err := os.Stat(dir); !os.IsNotExist(err) { - return + ready, err := probe() + if err != nil { + return err + } + if ready { + return nil + } + if !time.Now().Before(deadline) { + return fmt.Errorf("timed out after %s", timeout) + } + time.Sleep(interval) + } +} + +// waitDirExists waits for a required path without allowing an infinite boot +// hang if the mount/path never becomes available. +func waitDirExists(dir string, timeout time.Duration) error { + if err := waitUntil(timeout, time.Second, func() (bool, error) { + _, err := os.Stat(dir) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil } - time.Sleep(time.Second) + return false, err + }); err != nil { + return fmt.Errorf("wait for %s: %w", dir, err) } + return nil +} + +// waitProcess waits for pidof to report a process. pidof's normal "not found" +// exit status is treated as "not ready yet"; an absent pidof binary is a real +// platform error. +func waitProcess(name string, timeout time.Duration) error { + if err := waitUntil(timeout, time.Second, func() (bool, error) { + out, err := exec.Command("pidof", name).CombinedOutput() + if err == nil { + return len(bytes.TrimSpace(out)) > 0, nil + } + if _, ok := err.(*exec.ExitError); ok { + return false, nil + } + return false, err + }); err != nil { + return fmt.Errorf("wait for process %s: %w", name, err) + } + return nil } diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 057628cd..1713ce4a 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -2,6 +2,8 @@ package merlin import ( "bytes" + "errors" + "time" "strings" "testing" @@ -38,3 +40,41 @@ func Test_merlinParsePostConf(t *testing.T) { }) } } + + +func Test_waitUntilReady(t *testing.T) { + calls := 0 + err := waitUntil(100*time.Millisecond, time.Millisecond, func() (bool, error) { + calls++ + return calls >= 2, nil + }) + if err != nil { + t.Fatal(err) + } + if calls < 2 { + t.Fatalf("probe calls = %d, want at least 2", calls) + } +} + +func Test_waitUntilPropagatesProbeError(t *testing.T) { + want := errors.New("probe failed") + err := waitUntil(100*time.Millisecond, time.Millisecond, func() (bool, error) { + return false, want + }) + if !errors.Is(err, want) { + t.Fatalf("error = %v, want %v", err, want) + } +} + +func Test_waitUntilTimesOut(t *testing.T) { + start := time.Now() + err := waitUntil(10*time.Millisecond, time.Millisecond, func() (bool, error) { + return false, nil + }) + if err == nil { + t.Fatal("expected timeout") + } + if time.Since(start) > time.Second { + t.Fatal("bounded wait exceeded expected test duration") + } +}