sandbox: release the token only when it was acquired

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)

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)
 	})
 }