From 746119d5753945a2c0ddc4cc76256bab0884489c Mon Sep 17 00:00:00 2001 From: Marko Petzold Date: Thu, 29 Oct 2026 12:09:33 +0100 Subject: [PATCH 1/2] enhancement: improve frpc supervision with retry logic and backoff strategy --- src/agent.go | 5 +- src/tunnel/capability_test.go | 121 +++++++++++++++++++++++- src/tunnel/tunnel.go | 172 ++++++++++++++++++++++++++-------- 3 files changed, 253 insertions(+), 45 deletions(-) diff --git a/src/agent.go b/src/agent.go index 2b8e346..0119c1e 100644 --- a/src/agent.go +++ b/src/agent.go @@ -152,8 +152,9 @@ func (agent *Agent) OnConnect(reconnect bool) error { // frpc is delivered on every supported platform: embedded on Linux/macOS, // downloaded (separate signed binary) on Windows. If it can't be acquired - // or won't connect, SuperviseStart settles the device into an - // unavailable-but-alive state — the agent and app starts are unaffected. + // or won't connect, SuperviseStart keeps the device in an + // unavailable-but-alive state while it retries — the agent and app starts + // are unaffected. if !reconnect { // Before frpc starts: Windows cannot replace a running frpc.exe. err = acquireFrpcWithin(agent.System.DownloadFrpIfNotExists, frpcAcquireWait) diff --git a/src/tunnel/capability_test.go b/src/tunnel/capability_test.go index 5b02140..f7dfa09 100644 --- a/src/tunnel/capability_test.go +++ b/src/tunnel/capability_test.go @@ -407,6 +407,8 @@ func TestSuperviseStartRepicksAdminPortAfterPreLoginExit(t *testing.T) { require.NoError(t, err) defer ln.Close() + // The supervisor never gives up on this frpc; Stop ends it. + t.Cleanup(func() { _ = m.Stop() }) go m.SuperviseStart() require.Eventually(t, func() bool { @@ -444,6 +446,7 @@ func TestClientLostAfterStartIsRestarted(t *testing.T) { m := newCapabilityTestManager(t) m.loginDeadline = 200 * time.Millisecond + m.startRetryBackoff = time.Millisecond // lost within the retry interval: restarted after the backoff reachableFrps(t, m) // The first frpc outlives the login deadline and then dies; its @@ -487,10 +490,9 @@ func TestStoppedClientIsNotRestarted(t *testing.T) { "a stopped client must stay stopped") } -// A client that dies before Start hands it over is Start's failure, which -// SuperviseStart retries a bounded number of times. Its exit must not restart -// it as well: an frpc that antivirus ends at once would then be retried -// forever instead of settling unavailable. +// A client that dies before Start hands it over is Start's failure, which only +// SuperviseStart retries, with its backoff. Its exit must not restart it as +// well: an frpc that antivirus ends at once would then be retried without any. func TestClientDyingDuringStartIsNotRestartedByItsExit(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake frpc is a shell script") @@ -516,6 +518,117 @@ func TestClientDyingDuringStartIsNotRestartedByItsExit(t *testing.T) { assert.Equal(t, CapabilityUnavailable, cap) } +// The supervisor's pacing: the fast backoff, then the retry interval for good. +func TestStartRetryDelay(t *testing.T) { + m := newCapabilityTestManager(t) + + want := []time.Duration{2 * time.Second, 4 * time.Second, 8 * time.Second, 16 * time.Second, 32 * time.Second} + for i, delay := range want { + assert.Equal(t, delay, m.startRetryDelay(i+1), "failure %d", i+1) + } + for _, failures := range []int{fastStartAttempts, fastStartAttempts + 1, 1000} { + assert.Equal(t, 10*time.Minute, m.startRetryDelay(failures), "failure %d", failures) + } +} + +// Regression (LAC PC, 2026-10-29): Defender ends every frpc started in the +// minutes after a scan or a definitions update. The supervisor gave up after six +// such attempts and the tunnels stayed down until the agent was restarted, two +// hours later — though the next start after the window survived. It must keep +// trying, with the device unavailable meanwhile. +func TestSuperviseStartNeverGivesUp(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake frpc is a shell script") + } + + m := newCapabilityTestManager(t) + m.startRetryBackoff = time.Millisecond + m.startRetryInterval = 50 * time.Millisecond + + spawns := filepath.Join(t.TempDir(), "spawns") + fake := fmt.Sprintf("#!/bin/sh\necho x >> '%s'\nexit 1\n", spawns) + frpcPath := filesystem.GetTunnelBinaryPath(m.config, "frpc") // frpc.exe on Windows + require.NoError(t, os.WriteFile(frpcPath, []byte(fake), 0o755)) + + supervised := make(chan struct{}) + go func() { + defer close(supervised) + m.SuperviseStart() + }() + + require.Eventually(t, func() bool { return spawnCount(spawns) >= fastStartAttempts+3 }, 20*time.Second, 50*time.Millisecond, + "the supervisor must go on after the fast attempts") + + // Stop ends the supervisor, waiting or not. + require.NoError(t, m.Stop()) + select { + case <-supervised: + case <-time.After(5 * time.Second): + t.Fatal("Stop must end a supervisor waiting to retry") + } + cap, _ := m.Capability() + assert.Equal(t, CapabilityUnavailable, cap) +} + +// Regression (LAC PC, 2026-10-27): Defender let each frpc log in and ended it +// under a minute later. Handed over by then, every one was restarted at once — +// a new frpc, and a new Defender detection, every minute all night. A client +// lost before it ran for the retry interval must count as a failed start. +func TestClientLostSoonAfterStartBacksOff(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake frpc is a shell script") + } + + m := newCapabilityTestManager(t) + m.loginDeadline = 100 * time.Millisecond + m.startRetryBackoff = time.Millisecond + m.startRetryInterval = time.Minute // never reached: the test ends first + reachableFrps(t, m) + + // Every frpc outlives the login deadline and then dies. + spawns := filepath.Join(t.TempDir(), "spawns") + fake := fmt.Sprintf("#!/bin/sh\necho x >> '%s'\nsleep 0.5\n", spawns) + frpcPath := filesystem.GetTunnelBinaryPath(m.config, "frpc") // frpc.exe on Windows + require.NoError(t, os.WriteFile(frpcPath, []byte(fake), 0o755)) + t.Cleanup(func() { _ = m.Stop() }) + + go m.SuperviseStart() + + require.Eventually(t, func() bool { return spawnCount(spawns) == fastStartAttempts }, 20*time.Second, 50*time.Millisecond) + assert.Never(t, func() bool { return spawnCount(spawns) > fastStartAttempts }, 2*time.Second, 50*time.Millisecond, + "after the fast attempts a client that keeps dying waits the retry interval") + cap, _ := m.Capability() + assert.Equal(t, CapabilityUnavailable, cap, "nothing runs while the supervisor waits") +} + +// A client that ran for the retry interval worked: whatever ends it is a new +// problem, retried from the fast end — not after another interval because of +// failures before it ran. +func TestClientThatRanLongStartsTheFailureCountOver(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake frpc is a shell script") + } + + m := newCapabilityTestManager(t) + m.loginDeadline = 200 * time.Millisecond + m.startRetryInterval = 300 * time.Millisecond + reachableFrps(t, m) + m.startFailures.Store(fastStartAttempts) // the failures before it ran + + // The first frpc runs past the retry interval and then dies; its + // replacement stays up. + spawns := filepath.Join(t.TempDir(), "spawns") + fake := fmt.Sprintf("#!/bin/sh\nif [ -e '%[1]s' ]; then echo again >> '%[1]s'; exec sleep 30; fi\necho first >> '%[1]s'\nsleep 1\n", spawns) + frpcPath := filesystem.GetTunnelBinaryPath(m.config, "frpc") // frpc.exe on Windows + require.NoError(t, os.WriteFile(frpcPath, []byte(fake), 0o755)) + t.Cleanup(func() { _ = m.Stop() }) + + require.NoError(t, m.Start()) + + require.Eventually(t, func() bool { return spawnCount(spawns) == 2 }, 10*time.Second, 50*time.Millisecond) + assert.Zero(t, m.startFailures.Load(), "a client that ran for the retry interval starts the failure count over") +} + // SuperviseStart re-picks the admin port while a status publish reads it // (both run from AllStatus); the race detector catches it without the lock. func TestAdminPortIsSafeForConcurrentUse(t *testing.T) { diff --git a/src/tunnel/tunnel.go b/src/tunnel/tunnel.go index 2843d2f..d6fb450 100644 --- a/src/tunnel/tunnel.go +++ b/src/tunnel/tunnel.go @@ -79,6 +79,15 @@ const ( // to a reachability probe instead of declaring the device unavailable. const defaultLoginDeadline = 30 * time.Second +const ( + // fastStartAttempts is how many failures the supervisor retries on the short + // backoff (defaultStartRetryBackoff, doubling) before it settles on + // defaultStartRetryInterval (see startRetryDelay). + fastStartAttempts = 6 + defaultStartRetryBackoff = 2 * time.Second + defaultStartRetryInterval = 10 * time.Minute +) + const ( // frpsProbeTimeout bounds a single reachability dial to the frps control port. frpsProbeTimeout = 5 * time.Second @@ -124,7 +133,20 @@ type FrpTunnelManager struct { startMu sync.Mutex // supervising makes SuperviseStart single-flight; a second caller returns // instead of running a competing retry loop. - supervising atomic.Bool + supervising atomic.Bool + // startFailures counts the client's consecutive failures to start or to + // keep running (see countStartFailure); it paces the supervisor's next + // attempt (startRetryDelay). + startFailures atomic.Int32 + // startRetryBackoff and startRetryInterval pace those attempts (see + // startRetryDelay). Fields so tests can shorten them; zero falls back to + // the defaults. + startRetryBackoff time.Duration + startRetryInterval time.Duration + // supervisionStop is closed by Stop, which ends a supervisor waiting to + // retry: a client stopped on purpose stays stopped. Guarded by startMu. + supervisionStop chan struct{} + configBuilder TunnelConfigBuilder config *config.Config messenger messenger.Messenger @@ -163,12 +185,12 @@ type FrpTunnelManager struct { // frpcRun is the lifecycle of one spawned frpc process. Its exit restarts the // client only when Start had handed it over (established) and nobody stopped -// it on purpose. A client that dies earlier is Start's failure, which -// SuperviseStart retries a bounded number of times; restarting it from its -// exit as well would retry an frpc that cannot run (antivirus killing it at -// once) forever. +// it on purpose. A client that dies earlier is Start's failure, which only +// SuperviseStart retries; restarting it from its exit as well would retry an +// frpc that antivirus kills at once without any backoff. type frpcRun struct { mu sync.Mutex + spawned time.Time established bool stopped bool exited bool @@ -196,14 +218,14 @@ func (run *frpcRun) stop() { run.stopped = true } -// exit records the exit and reports whether the client was lost: established -// and not stopped on purpose. -func (run *frpcRun) exit() bool { +// exit records the exit. It reports whether the client was lost (established +// and not stopped on purpose) and how long it ran. +func (run *frpcRun) exit() (lost bool, ranFor time.Duration) { run.mu.Lock() defer run.mu.Unlock() run.exited = true - return run.established && !run.stopped + return run.established && !run.stopped, time.Since(run.spawned) } // errFrpcExitedDuringStart fails a Start whose client died before Start could @@ -303,8 +325,8 @@ type TunnelManager interface { Status(tunnelID string) (TunnelStatus, error) Reload() error Start() error - // SuperviseStart brings the client up with bounded ret/backoff; on - // exhaustion the device settles into an unavailable-but-alive state. + // SuperviseStart brings the client up and keeps retrying until it runs; + // meanwhile the device is in an unavailable-but-alive state. SuperviseStart() SaveRemotePorts(payload common.TransitionPayload) error // TunnelCapable reports whether tunnels can run on this device (true unless @@ -948,12 +970,26 @@ func (frpTm *FrpTunnelManager) stopClientLocked() { // restart paths in AllStatus and Reload match a "connection refused" that // Windows words differently — so the tunnels stayed down until the agent // restarted. +// +// A client that ran for the retry interval worked, so whatever ends it (or a +// deliberate stop) starts the failure count over. One lost sooner counts as a +// failed start. Restarted at once instead, an frpc that antivirus lets log in +// and ends under a minute later came back every minute: on the LAC PC that was +// a Defender detection a minute, all night (2026-10-27). func (frpTm *FrpTunnelManager) onClientExit(run *frpcRun) { - if !run.exit() { + lost, ranFor := run.exit() + worked := ranFor >= frpTm.retryInterval() + if worked { + frpTm.startFailures.Store(0) + } + if !lost { return } - log.Warn().Msg("the tunnel client exited unexpectedly; restarting it") + if !worked { + frpTm.countStartFailure() + } + log.Warn().Dur("ranFor", ranFor).Msg("the tunnel client exited unexpectedly; restarting it") safe.Go(frpTm.SuperviseStart) } @@ -1063,7 +1099,7 @@ func (frpTm *FrpTunnelManager) Start() error { } clientDone := make(chan struct{}) - run := &frpcRun{} + run := &frpcRun{spawned: time.Now()} frpTm.clientProcess = frpCommand frpTm.clientDone = clientDone frpTm.clientRun = run @@ -1097,8 +1133,10 @@ func (frpTm *FrpTunnelManager) Start() error { // No process left to speak for: park the capability probe until the // supervisor spawns a replacement. defer frpTm.clientAlive.Store(false) - // If the loop exits without a prior ack, frpc died before login. - defer ack(errors.New("frpc exited before logging in to the server")) + // If the loop exits without a prior ack, frpc died during Start. Not + // necessarily before it logged in: its login line goes to the log + // file, not to this pipe (see defaultLoginDeadline). + defer ack(errFrpcExitedDuringStart) scanner := bufio.NewScanner(stdout) @@ -1278,11 +1316,12 @@ func (frpTm *FrpTunnelManager) Start() error { } } -// SuperviseStart brings the tunnel client up with bounded exponential backoff. -// It replaces "call Start() once and hope": a permanently-failing frpc (missing -// binary that can't be re-acquired, repeated login failures) settles into -// CapabilityUnavailable instead of hanging or spinning forever, so the device -// degrades cleanly. Runs to completion (success or exhaustion) — call it from a +// SuperviseStart brings the tunnel client up and keeps trying until it runs, +// paced by startRetryDelay; the device reports CapabilityUnavailable while it +// fails, so it degrades cleanly instead of hanging. It used to give up after six +// attempts, which left a Windows PC whose antivirus had ended frpc for a few +// minutes without tunnels until the agent restarted (LAC PC, 2026-10-29). +// Returns once a client is handed over, or when Stop ends it — call it from a // goroutine. func (frpTm *FrpTunnelManager) SuperviseStart() { // Single-flight: the boot path, a failed Reload and a dead admin API can @@ -1296,39 +1335,91 @@ func (frpTm *FrpTunnelManager) SuperviseStart() { } defer frpTm.supervising.Store(false) - const maxAttempts = 6 - backoff := 2 * time.Second + frpTm.startMu.Lock() + stopped := frpTm.supervisionStop + frpTm.startMu.Unlock() + + for { + if failures := int(frpTm.startFailures.Load()); failures > 0 { + // A client lost soon after it started (onClientExit) left the + // capability Available, though nothing runs while this waits. + if failures >= fastStartAttempts { + if capability, _ := frpTm.Capability(); capability != CapabilityUnavailable { + frpTm.setCapability(CapabilityUnavailable, errors.New("the tunnel client keeps exiting soon after it starts")) + } + } + + select { + case <-time.After(frpTm.startRetryDelay(failures)): + case <-stopped: + return + } + } - for attempt := 1; attempt <= maxAttempts; attempt++ { err := frpTm.Start() if err == nil { log.Info().Msg("tunnel client started") return } - log.Warn().Err(err).Msgf("tunnel client start attempt %d/%d failed", attempt, maxAttempts) - if attempt < maxAttempts { - // A pre-login exit is regularly the admin port having been taken - // while frpc was down (any outbound localhost connection can hold - // it). Retrying on the same port then fails identically all the - // way to exhaustion — re-pick a free one for the next attempt. - frpTm.configBuilder.SetAdminPort() - - time.Sleep(backoff) - if backoff < 32*time.Second { - backoff *= 2 - } - } + failures := frpTm.countStartFailure() + log.Warn().Err(err).Msgf("tunnel client start attempt failed (%d in a row); retrying in %s", failures, frpTm.startRetryDelay(failures)) + + // A pre-login exit is regularly the admin port having been taken + // while frpc was down (any outbound localhost connection can hold + // it). Retrying on the same port then fails identically every time + // — re-pick a free one for the next attempt. + frpTm.configBuilder.SetAdminPort() + } +} + +// startRetryDelay is the supervisor's wait after failures (at least one) +// consecutive failures: the backoff doubling from startRetryBackoff (2 s to +// 32 s) for the first fastStartAttempts, then the retry interval, for as long +// as it takes. +// Antivirus that ends frpc does so in the minutes after a scan or a definitions +// update, and starts outside that window survive; an interval longer than the +// window lets the next attempt outlive it without a new detection a minute. +func (frpTm *FrpTunnelManager) startRetryDelay(failures int) time.Duration { + if failures >= fastStartAttempts { + return frpTm.retryInterval() } - frpTm.setCapability(CapabilityUnavailable, errors.New("tunnel client failed to start after repeated attempts")) - log.Error().Msg("giving up starting the tunnel client; tunnels are unavailable on this device") + backoff := frpTm.startRetryBackoff + if backoff <= 0 { + backoff = defaultStartRetryBackoff + } + return backoff << (failures - 1) +} + +// retryInterval is how often the supervisor retries after the fast attempts, +// and how long a client has to run to count as having worked (onClientExit). +func (frpTm *FrpTunnelManager) retryInterval() time.Duration { + if frpTm.startRetryInterval <= 0 { + return defaultStartRetryInterval + } + return frpTm.startRetryInterval +} + +// countStartFailure counts a client that failed to start or did not run for +// the retry interval, and returns the count. +func (frpTm *FrpTunnelManager) countStartFailure() int { + failures := int(frpTm.startFailures.Add(1)) + if failures == fastStartAttempts { + log.Error().Msgf("the tunnel client keeps failing; tunnels are unavailable on this device, retrying every %s", frpTm.retryInterval()) + } + return failures } func (frpTm *FrpTunnelManager) Stop() error { frpTm.startMu.Lock() defer frpTm.startMu.Unlock() + if frpTm.supervisionStop != nil { + close(frpTm.supervisionStop) + } + frpTm.supervisionStop = make(chan struct{}) + frpTm.stopClientLocked() return nil } @@ -1390,6 +1481,9 @@ func NewFrpTunnelManager(messenger messenger.Messenger, config *config.Config) ( loginChan: make(chan bool, 10), // Buffered to avoid blocking isLoggedIn: false, loginDeadline: defaultLoginDeadline, + startRetryBackoff: defaultStartRetryBackoff, + startRetryInterval: defaultStartRetryInterval, + supervisionStop: make(chan struct{}), } return frpTunnelManager, nil From 78642bd0597e0750bd537fbae177d26a09971700 Mon Sep 17 00:00:00 2001 From: Marko Petzold Date: Thu, 29 Oct 2026 12:09:41 +0100 Subject: [PATCH 2/2] release reagent v1.4.8 --- availableVersions.json | 8 ++++---- src/release/version.txt | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/availableVersions.json b/availableVersions.json index a3e5279..29f4cea 100644 --- a/availableVersions.json +++ b/availableVersions.json @@ -1,6 +1,6 @@ { - "production": "1.4.7", - "test": "1.4.7", - "local": "1.4.7", - "all": "1.4.7" + "production": "1.4.8", + "test": "1.4.8", + "local": "1.4.8", + "all": "1.4.8" } diff --git a/src/release/version.txt b/src/release/version.txt index b000a6a..721b993 100644 --- a/src/release/version.txt +++ b/src/release/version.txt @@ -1 +1 @@ -1.4.7 \ No newline at end of file +1.4.8 \ No newline at end of file