From e06a6244c8314db7545cee28e412c48c571464e8 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 08:15:02 +0000 Subject: [PATCH] fix(sandbox): clean up and back off after a failed idle suspend The sweep only logged a failed suspend and tried again every minute, holding the entry for up to a minute each time. When the guest was gone the entry kept pointing at the dead sandbox, so the chat's bash failed until the sandbox reached its maximum age. Drop the sandbox on ErrGone so the next call makes a fresh one. For any other error, wait longer after each failure (two minutes, doubling to thirty) and reset the wait on the next call. The log now names the user and conversation. Assisted-by: Claude Code:claude-sonnet-5-5 Signed-off-by: Ettore Di Giacinto --- core/services/studio/sandbox/runner/runner.go | 33 ++++- .../studio/sandbox/runner/sweep_test.go | 113 ++++++++++++++++++ 2 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 core/services/studio/sandbox/runner/sweep_test.go diff --git a/core/services/studio/sandbox/runner/runner.go b/core/services/studio/sandbox/runner/runner.go index 5584687d5..4c3006056 100644 --- a/core/services/studio/sandbox/runner/runner.go +++ b/core/services/studio/sandbox/runner/runner.go @@ -116,6 +116,11 @@ type entry struct { trk *tracker gone bool hasBuilt bool + // suspendFails counts the failed suspends since the last call, and + // suspendAfter is the earliest time the sweep tries again. Both are + // guarded by mu. + suspendFails int + suspendAfter time.Time // mounts are the folders of the sandbox's view, to name a path of a // commit as the guest sees it. A resumed sandbox has the view it was // made with. @@ -479,6 +484,7 @@ func (m *Manager) Run(ctx context.Context, r Request) (string, error) { end = m.o.Begin(ctx, r.User, r.Conversation, r.Operation) defer end() // a panic of the tool still ends the call } + e.suspendFails, e.suspendAfter = 0, time.Time{} out, runErr := tool.Run(ctx, r.Args) // The call's changes become one change set, whether or not the command // succeeded: a failing script may still have written files. @@ -496,6 +502,14 @@ func (m *Manager) Run(ctx context.Context, r Request) (string, error) { return out, runErr } +// suspendBackoff is the wait after n failed suspends of one sandbox: the +// sweep runs every minute, and a suspend that fails holds the entry for up to +// a minute, so it doubles from two minutes to half an hour. +func suspendBackoff(n int) time.Duration { + d := time.Minute << min(n, 5) + return min(d, 30*time.Minute) +} + // Sweep puts idle sandboxes to sleep and removes the old ones. It is called // once a minute; tests call it by hand. func (m *Manager) Sweep(ctx context.Context) { @@ -518,14 +532,25 @@ func (m *Manager) Sweep(ctx context.Context) { e.gone = true m.destroy(ctx, e) m.forget(e) - case e.sb != nil && idle >= m.o.IdleAfter: + case e.sb != nil && idle >= m.o.IdleAfter && !now.Before(e.suspendAfter): sctx, cancel := context.WithTimeout(ctx, time.Minute) ref, err := e.sb.Suspend(sctx) cancel() - if err != nil { - xlog.Warn("sandbox: could not suspend an idle sandbox", "error", err) - } else { + switch { + case err == nil: e.sb, e.snap = nil, &ref + e.suspendFails = 0 + case errors.Is(err, sandbox.ErrGone): + // The guest stopped by itself: keeping it would send every + // next call of the chat to a dead sandbox. + xlog.Warn("sandbox: an idle sandbox is gone, dropping it", "user", e.user, "conversation", e.conv, "error", err) + m.destroy(ctx, e) + e.suspendFails = 0 + default: + e.suspendFails++ + e.suspendAfter = now.Add(suspendBackoff(e.suspendFails)) + xlog.Warn("sandbox: could not suspend an idle sandbox, trying again later", "user", e.user, "conversation", e.conv, + "failures", e.suspendFails, "retry_after", e.suspendAfter, "error", err) } } e.mu.Unlock() diff --git a/core/services/studio/sandbox/runner/sweep_test.go b/core/services/studio/sandbox/runner/sweep_test.go new file mode 100644 index 000000000..3ade3af0d --- /dev/null +++ b/core/services/studio/sandbox/runner/sweep_test.go @@ -0,0 +1,113 @@ +//go:build linux || darwin + +package runner_test + +import ( + "context" + "errors" + "sync" + "time" + + "github.com/mudler/LocalAI/core/services/studio/sandbox" + "github.com/mudler/LocalAI/core/services/studio/sandbox/runner" + "github.com/mudler/LocalAI/core/services/studio/sandbox/sbtest" + "github.com/mudler/LocalAI/core/services/studio/workspace" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// flakyBackend makes sandboxes whose Suspend fails with err, and counts the +// sandboxes it made and the suspends it saw. +type flakyBackend struct { + sandbox.Backend + mu sync.Mutex + err error + created int + suspends int +} + +func (b *flakyBackend) Create(ctx context.Context, spec sandbox.Spec) (sandbox.Sandbox, error) { + sb, err := b.Backend.Create(ctx, spec) + if err != nil { + return nil, err + } + b.mu.Lock() + b.created++ + b.mu.Unlock() + return &flakySandbox{Sandbox: sb, b: b}, nil +} + +func (b *flakyBackend) counts() (created, suspends int) { + b.mu.Lock() + defer b.mu.Unlock() + return b.created, b.suspends +} + +type flakySandbox struct { + sandbox.Sandbox + b *flakyBackend +} + +func (s *flakySandbox) Suspend(context.Context) (sandbox.SnapshotRef, error) { + s.b.mu.Lock() + defer s.b.mu.Unlock() + s.b.suspends++ + return sandbox.SnapshotRef{}, s.b.err +} + +var _ = Describe("the sweep of idle sandboxes", func() { + var ( + rig *scopedRig + fb *flakyBackend + m *runner.Manager + now time.Time + ) + BeforeEach(func() { + rig = newScopedRig(workspace.ModeWrite) + fb = &flakyBackend{Backend: sbtest.NewFakeBackend(GinkgoT().TempDir())} + now = time.Now() + var err error + m, err = runner.New(runner.Options{ + Backend: func() (sandbox.Backend, error) { return fb, nil }, + Store: rig.st, + Actor: rig.actor, + View: rig.view, + IdleAfter: time.Minute, + Now: func() time.Time { return now }, + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { _ = m.Close(context.Background()) }) + rig.m = m + rig.must("echo one") + now = now.Add(2 * time.Minute) + }) + + It("drops a sandbox whose guest is gone, so the next call makes a fresh one", func() { + fb.err = errors.Join(sandbox.ErrGone, errors.New("the guest exited")) + m.Sweep(context.Background()) + Expect(rig.must("echo two")).To(ContainSubstring("two")) + created, _ := fb.counts() + Expect(created).To(Equal(2)) + }) + + for name, cause := range map[string]error{ + "a failed checkpoint": errors.New("remote: no request is being served"), + "a busy sandbox": sandbox.ErrBusy, + } { + It("waits before it suspends again after "+name+", and keeps the sandbox usable", func() { + fb.err = cause + m.Sweep(context.Background()) + now = now.Add(time.Minute) + m.Sweep(context.Background()) + _, suspends := fb.counts() + Expect(suspends).To(Equal(1), "the next sweep a minute later does not try again") + now = now.Add(time.Hour) + m.Sweep(context.Background()) + _, suspends = fb.counts() + Expect(suspends).To(Equal(2), "it tries again after the wait") + Expect(rig.must("echo three")).To(ContainSubstring("three")) + created, _ := fb.counts() + Expect(created).To(Equal(1), "the sandbox that could not sleep still serves") + }) + } +})