From 874b962cd773746502e7ee209f2214529bec95ce Mon Sep 17 00:00:00 2001 From: Nikolay Tuzov Date: Fri, 21 Aug 2026 14:02:08 +0500 Subject: [PATCH] sandbox: release the token only when it was acquired The deferred Release was registered above the ErrBusy check, so a rejected request put back a token it never took and the next caller started a worker beyond pool_size. With pool_size 1, a burst of 8 requests ran 4 containers side by side. Tokens do not accumulate, so this is not a leak: Release is non-blocking and the channel holds at most pool_size of them. The excess lives inside a burst, where every rejection hands out one more slot, so the ceiling follows the load instead of the setting. The doc comment on Exec already promises no more than pool.Size() concurrent workers, so this brings the code to what is documented rather than changing the design. --- internal/sandbox/sandbox.go | 5 ++--- internal/sandbox/sandbox_test.go | 9 +++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 5654d33..709d053 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -34,11 +34,10 @@ func Validate(in engine.Request) error { // Allows no more than pool.Size() concurrent workers at any given time. // The request must already be validated by Validate(). func Exec(in engine.Request) engine.Execution { - err := semaphore.Acquire() - defer semaphore.Release() - if err == ErrBusy { + if err := semaphore.Acquire(); err == ErrBusy { return engine.Fail(in.ID, engine.ErrBusy) } + defer semaphore.Release() start := time.Now() engine := engines[in.Sandbox][in.Command] out := engine.Exec(in) diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index fa0af69..3ab492f 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -74,8 +74,11 @@ func TestExec(t *testing.T) { be.Equal(t, out.Stdout, "hello") be.Equal(t, out.Stderr, "") be.Equal(t, out.Err, nil) + // the worker returns its token when it is done + be.Equal(t, semaphore.Size(), cfg.PoolSize) }) t.Run("busy", func(t *testing.T) { + defer func() { _ = ApplyConfig(cfg) }() for i := 0; i < cfg.PoolSize; i++ { _ = semaphore.Acquire() } @@ -89,5 +92,11 @@ func TestExec(t *testing.T) { } out := Exec(req) be.Err(t, out.Err, engine.ErrBusy) + // a rejected request must not release a token it never acquired + be.Equal(t, semaphore.Size(), 0) + // otherwise the next request picks up the donated token + // and runs while the pool is still fully occupied + out = Exec(req) + be.Err(t, out.Err, engine.ErrBusy) }) }