fix(api): stop an in-flight resume from undoing a paused sandbox delete

packages/api/internal/handlers/sandbox_kill.go

@@ -65,9 +65,21 @@ func (a *APIStore) DeleteSandboxesSandboxID(
 		Action: sandbox.StateActionKill,
 		Reason: sandbox.KillReasonRequest,
 	})
+
+	// runningRemoved records whether RemoveSandbox killed a running record. When
+	// it did not (the sandbox is paused: only a snapshot exists, no running
+	// record to lock), deleting the snapshot below races a concurrent resume,
+	// whose publication (storage.Add) is a lockless SET+SADD with no
+	// delete-intent check. Without a rendezvous the DELETE can soft-delete the
+	// snapshot and return 204 while the resume republishes the sandbox as
+	// running. We fence that window with a kill-claim on the reservation the
+	// resume holds for its whole lifecycle.
+	runningRemoved := false
+
 	switch {
 	case err == nil:
 		killedOrRemoved = true
+		runningRemoved = true
 	case errors.Is(err, orchestrator.ErrSandboxNotFound):
 		logger.L().Debug(ctx, "Running sandbox not found", logger.WithSandboxID(sandboxID))
 	case errors.Is(err, orchestrator.ErrSandboxOperationFailed):
@@ -81,12 +93,41 @@ func (a *APIStore) DeleteSandboxesSandboxID(
 		return
 	}
 
+	// Paused sandbox: claim the ID against a concurrent resume before touching
+	// the snapshot. If a resume is in flight (or already finished, so the
+	// sandbox is running again), refuse with 409 and leave the snapshot intact —
+	// the client retries the kill against the running sandbox through the locked
+	// path above. An accepted kill (a claim) is irreversible: reserveScript
+	// rejects any resume that starts after it.
+	claimTaken := false
+	if !runningRemoved {
+		claimed, claimErr := a.orchestrator.ClaimPausedKill(ctx, teamID, sandboxID)
+		if claimErr != nil {
+			telemetry.ReportError(ctx, "error claiming paused sandbox for deletion", claimErr)
+			a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error killing sandbox: %s", claimErr))
+
+			return
+		}
+		if !claimed {
+			logger.L().Info(ctx, "Refusing to delete paused sandbox: a resume is in flight", logger.WithSandboxID(sandboxID))
+			a.sendAPIStoreError(c, http.StatusConflict, fmt.Sprintf("Sandbox %s is resuming; retry the delete once it is running", sandboxID))
+
+			return
+		}
+		claimTaken = true
+	}
+
 	// remove any snapshots when the sandbox is not running
 	deleteSnapshotErr := a.deleteSnapshot(ctx, sandboxID, teamID)
 	switch {
 	case errors.Is(deleteSnapshotErr, db.ErrSnapshotNotFound):
 		// no snapshot found, nothing to do
 	case deleteSnapshotErr != nil:
+		if claimTaken {
+			// The snapshot survived, so drop the claim to unblock future resumes
+			// of this ID rather than making them wait out the claim's TTL.
+			a.orchestrator.ReleasePausedKillClaim(context.WithoutCancel(ctx), teamID, sandboxID)
+		}
 		telemetry.ReportError(ctx, "error deleting sandbox", deleteSnapshotErr)
 		a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error deleting sandbox: %s", deleteSnapshotErr))
 

packages/api/internal/orchestrator/create_instance.go

@@ -207,6 +207,16 @@ func (o *Orchestrator) CreateSandbox(
 						"please visit 'https://e2b.dev/docs/billing'", totalConcurrentInstances),
 				Err: fmt.Errorf("team '%s' has reached the maximum number of instances (%d)", team.ID, totalConcurrentInstances),
 			}
+		case errors.Is(err, sandbox.ErrSandboxKilled):
+			// A DELETE claimed this sandbox ID for removal while this resume was
+			// starting. The kill wins: the snapshot is being (or has been)
+			// deleted, so publishing this sandbox would resurrect a sandbox the
+			// client was told was gone. Refuse instead.
+			return sandbox.Sandbox{}, &api.APIError{
+				Code:      http.StatusNotFound,
+				ClientMsg: fmt.Sprintf("Sandbox '%s' was deleted", sandboxID),
+				Err:       fmt.Errorf("resume of '%s' refused: %w", sandboxID, err),
+			}
 		default:
 			logger.L().Error(ctx, "failed to reserve sandbox for team", logger.WithSandboxID(sandboxID), zap.Error(err))
 

packages/api/internal/orchestrator/delete_instance.go

@@ -29,6 +29,23 @@ const refusalRetryAfter = 10 * time.Second
 
 const pauseTimeout = 80 * time.Second
 
+// ClaimPausedKill fences a paused sandbox's ID against a concurrent resume
+// before the caller deletes its snapshot. It returns claimed=true when no
+// resume is in flight and the caller may proceed; false when a resume is
+// pending or the sandbox is already running again, in which case the snapshot
+// must be left intact and the kill retried against the running sandbox.
+func (o *Orchestrator) ClaimPausedKill(ctx context.Context, teamID uuid.UUID, sandboxID string) (bool, error) {
+	return o.sandboxStore.ClaimKill(ctx, teamID, sandboxID)
+}
+
+// ReleasePausedKillClaim drops a claim taken by ClaimPausedKill. Best-effort:
+// the claim also expires on its own.
+func (o *Orchestrator) ReleasePausedKillClaim(ctx context.Context, teamID uuid.UUID, sandboxID string) {
+	if err := o.sandboxStore.ReleaseKillClaim(ctx, teamID, sandboxID); err != nil {
+		logger.L().Error(ctx, "failed to release paused-kill claim", zap.Error(err), logger.WithSandboxID(sandboxID))
+	}
+}
+
 func (o *Orchestrator) RemoveSandbox(ctx context.Context, teamID uuid.UUID, sandboxID string, opts sandbox.RemoveOpts) error {
 	ctx, span := tracer.Start(ctx, "remove-sandbox")
 	defer span.End()

packages/api/internal/sandbox/aliases.go

@@ -59,6 +59,7 @@ var (
 	ErrRestoreConflict    = sandboxtypes.ErrRestoreConflict
 	ErrTransitionRestored = sandboxtypes.ErrTransitionRestored
 	ErrDraining           = sandboxtypes.ErrDraining
+	ErrSandboxKilled      = sandboxtypes.ErrSandboxKilled
 
 	AllowedTransitions = sandboxtypes.AllowedTransitions
 

packages/api/internal/sandbox/reservations/redis/kill_claim_test.go

@@ -0,0 +1,139 @@
+package redis
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/e2b-dev/infra/packages/api/internal/sandbox/sandboxtypes"
+	storage_redis "github.com/e2b-dev/infra/packages/api/internal/sandbox/storage/redis"
+)
+
+// TestClaimKill_NoResumeInFlight_ClaimsAndBlocksResume covers the common case:
+// a paused sandbox with no in-flight resume. ClaimKill succeeds, and any resume
+// that starts afterwards is refused so the accepted kill cannot be undone.
+func TestClaimKill_NoResumeInFlight_ClaimsAndBlocksResume(t *testing.T) {
+	t.Parallel()
+	storage, _ := setupTestReservationStorage(t)
+
+	teamID := uuid.New()
+
+	claimed, err := storage.ClaimKill(t.Context(), teamID, testSandboxID)
+	require.NoError(t, err)
+	assert.True(t, claimed, "kill should be claimed when no resume is in flight")
+
+	// A resume that starts after the claim must lose.
+	_, _, err = storage.Reserve(t.Context(), teamID, testSandboxID, 10)
+	require.ErrorIs(t, err, sandboxtypes.ErrSandboxKilled)
+}
+
+// TestClaimKill_ResumePending_Refuses covers the race the fix targets: a resume
+// is already mid-flight (it holds the reservation) when the DELETE arrives.
+// ClaimKill must refuse so the handler returns 409 and leaves the snapshot
+// intact, rather than deleting it and letting the resume resurrect the sandbox.
+func TestClaimKill_ResumePending_Refuses(t *testing.T) {
+	t.Parallel()
+	storage, _ := setupTestReservationStorage(t)
+
+	teamID := uuid.New()
+
+	finishStart, _, err := storage.Reserve(t.Context(), teamID, testSandboxID, 10)
+	require.NoError(t, err)
+	require.NotNil(t, finishStart, "first reserve should win the reservation")
+
+	claimed, err := storage.ClaimKill(t.Context(), teamID, testSandboxID)
+	require.NoError(t, err)
+	assert.False(t, claimed, "kill must be refused while a resume is pending")
+}
+
+// TestClaimKill_AlreadyRunning_Refuses covers the resume that finished between
+// StartRemoving finding no running record and the claim: the sandbox is back in
+// the storage index. ClaimKill must refuse so the client retries the kill
+// against the running sandbox via the normal locked path.
+func TestClaimKill_AlreadyRunning_Refuses(t *testing.T) {
+	t.Parallel()
+	storage, client := setupTestReservationStorage(t)
+
+	teamID := uuid.New()
+
+	// Simulate a completed resume: the sandbox is present in the storage index,
+	// which is what storage.Add does on publication.
+	indexKey := storage_redis.GetSandboxStorageTeamIndexKey(teamID.String())
+	require.NoError(t, client.SAdd(t.Context(), indexKey, testSandboxID).Err())
+
+	claimed, err := storage.ClaimKill(t.Context(), teamID, testSandboxID)
+	require.NoError(t, err)
+	assert.False(t, claimed, "kill must be refused when the sandbox is already running")
+}
+
+// TestReleaseKillClaim_UnblocksResume covers the cleanup path: when the snapshot
+// delete fails after a claim was taken, releasing the claim lets future resumes
+// of that ID proceed instead of waiting out the claim TTL.
+func TestReleaseKillClaim_UnblocksResume(t *testing.T) {
+	t.Parallel()
+	storage, _ := setupTestReservationStorage(t)
+
+	teamID := uuid.New()
+
+	claimed, err := storage.ClaimKill(t.Context(), teamID, testSandboxID)
+	require.NoError(t, err)
+	require.True(t, claimed)
+
+	// While claimed, a resume is refused.
+	_, _, err = storage.Reserve(t.Context(), teamID, testSandboxID, 10)
+	require.ErrorIs(t, err, sandboxtypes.ErrSandboxKilled)
+
+	require.NoError(t, storage.ReleaseKillClaim(t.Context(), teamID, testSandboxID))
+
+	// After release, the same ID can be reserved again.
+	finishStart, _, err := storage.Reserve(t.Context(), teamID, testSandboxID, 10)
+	require.NoError(t, err)
+	assert.NotNil(t, finishStart)
+}
+
+// TestClaimKill_ConcurrentReserveAndClaim asserts the rendezvous is atomic:
+// racing a resume's Reserve against a DELETE's ClaimKill, the two outcomes are
+// always consistent — exactly one of "resume reserved" / "kill claimed" wins,
+// never both, so a claimed kill is never resurrected and a reserved resume is
+// never silently killed.
+func TestClaimKill_ConcurrentReserveAndClaim(t *testing.T) {
+	t.Parallel()
+	storage, _ := setupTestReservationStorage(t)
+
+	for i := range 50 {
+		teamID := uuid.New()
+		sandboxID := "sbx-" + teamID.String()
+
+		var (
+			reserved bool
+			claimed  bool
+		)
+
+		done := make(chan struct{}, 2)
+		go func() {
+			finishStart, _, err := storage.Reserve(t.Context(), teamID, sandboxID, 10)
+			if err == nil && finishStart != nil {
+				reserved = true
+			}
+			done <- struct{}{}
+		}()
+		go func() {
+			c, err := storage.ClaimKill(t.Context(), teamID, sandboxID)
+			if err == nil {
+				claimed = c
+			}
+			done <- struct{}{}
+		}()
+		<-done
+		<-done
+
+		// If the kill was claimed, the resume must not have reserved (it either
+		// lost the race and was refused, or has not started). If the resume
+		// reserved first, the claim must have been refused.
+		if claimed && reserved {
+			t.Fatalf("iteration %d: both resume reserved and kill claimed for the same sandbox", i)
+		}
+	}
+}

packages/api/internal/sandbox/reservations/redis/reservation.go

@@ -25,6 +25,14 @@ const (
 	// and cleaned up. This handles the case where an API instance crashes mid-creation.
 	// 90 seconds is well beyond any realistic sandbox creation time.
 	staleTTL = 90 * time.Second
+
+	// killClaimTTL is how long a DELETE's kill-claim over a sandbox ID blocks a
+	// resume's reservation. It only has to outlive the snapshot soft-delete
+	// becoming durable — after that any resume fails when it fetches the
+	// snapshot — so it is kept short. The claim is normally cleared explicitly
+	// (ReleaseKillClaim) if the delete fails; this TTL is the backstop for a
+	// crashed API instance.
+	killClaimTTL = 30 * time.Second
 )
 
 var _ sandboxtypes.ReservationStorage = (*ReservationStorage)(nil)
@@ -56,8 +64,10 @@ func (s *ReservationStorage) Reserve(ctx context.Context, teamID uuid.UUID, sand
 	now := float64(time.Now().Unix())
 	staleCutoff := float64(time.Now().Add(-staleTTL).Unix())
 
+	killClaimKey := getKillClaimKey(teamIDStr, sandboxID)
+
 	result, err := reserveScript.Run(ctx, s.redisClient,
-		[]string{storageIndexKey, pendingSetKey, resultKeyStr},
+		[]string{storageIndexKey, pendingSetKey, resultKeyStr, killClaimKey},
 		sandboxID, limit, now, staleCutoff,
 	).Int()
 	if err != nil {
@@ -77,11 +87,60 @@ func (s *ReservationStorage) Reserve(ctx context.Context, teamID uuid.UUID, sand
 	case reserveResultLimitExceeded:
 		return nil, nil, &sandboxtypes.LimitExceededError{TeamID: teamID}
 
+	case reserveResultKilled:
+		return nil, nil, sandboxtypes.ErrSandboxKilled
+
 	default:
 		return nil, nil, fmt.Errorf("unexpected reserve script result: %d", result)
 	}
 }
 
+// ClaimKill fences off a paused sandbox's ID against a concurrent resume before
+// the caller soft-deletes its snapshot. It returns claimed=true when no resume
+// is in flight and the caller may proceed with the delete; false when a resume
+// is pending or the sandbox is already running again, in which case the caller
+// must not delete the snapshot and should surface a retryable conflict.
+//
+// See claimKillScript for the ordering argument that makes an accepted kill
+// irreversible against resume's lockless publication.
+func (s *ReservationStorage) ClaimKill(ctx context.Context, teamID uuid.UUID, sandboxID string) (claimed bool, err error) {
+	teamIDStr := teamID.String()
+	storageIndexKey := getStorageIndexKey(teamIDStr)
+	pendingSetKey := getPendingSetKey(teamIDStr)
+	killClaimKey := getKillClaimKey(teamIDStr, sandboxID)
+
+	result, err := claimKillScript.Run(ctx, s.redisClient,
+		[]string{storageIndexKey, pendingSetKey, killClaimKey},
+		sandboxID, int(killClaimTTL.Seconds()),
+	).Int()
+	if err != nil {
+		return false, fmt.Errorf("failed to run claim-kill script: %w", err)
+	}
+
+	switch result {
+	case claimKillResultClaimed:
+		return true, nil
+	case claimKillResultInFlight:
+		return false, nil
+	default:
+		return false, fmt.Errorf("unexpected claim-kill script result: %d", result)
+	}
+}
+
+// ReleaseKillClaim drops a claim taken by ClaimKill. It is best-effort cleanup
+// for when the snapshot delete fails after the claim was taken; the claim's TTL
+// is the backstop if this never runs.
+func (s *ReservationStorage) ReleaseKillClaim(ctx context.Context, teamID uuid.UUID, sandboxID string) error {
+	killClaimKey := getKillClaimKey(teamID.String(), sandboxID)
+
+	err := releaseKillClaimScript.Run(ctx, s.redisClient, []string{killClaimKey}).Err()
+	if err != nil {
+		return fmt.Errorf("failed to run release-kill-claim script: %w", err)
+	}
+
+	return nil
+}
+
 func (s *ReservationStorage) Release(ctx context.Context, teamID uuid.UUID, sandboxID string) error {
 	teamIDStr := teamID.String()
 	pendingSetKey := getPendingSetKey(teamIDStr)