fix(redis): stale sandboxes in killing state#2388
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 55150ca. Bugbot is set up for automated code reviews on this repo. Configure here. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
jakubno
requested review from
sitole
and removed request for
ValentaTomas and
dobrac
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1032a5ebf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if time.Since(sbx.EndTime) > sandbox.StaleCutoff && opts.Action.Effect == sandbox.TransitionExpires { | ||
| o.sandboxStore.Remove(context.WithoutCancel(ctx), teamID, sandboxID) | ||
| go o.analyticsRemove(context.WithoutCancel(ctx), sbx, opts.Action) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| defer func() { go o.analyticsRemove(context.WithoutCancel(ctx), sbx, opts.Action) }() | ||
| defer o.sandboxStore.Remove(ctx, teamID, sandboxID) | ||
| // Once we start the removal process, we want to make sure it gets removed from the store | ||
| defer o.sandboxStore.Remove(context.WithoutCancel(ctx), teamID, sandboxID) |
There was a problem hiding this comment.
Isn't it worth creating an anonymous cleanup function calling sandbox store, remove, and analytics remove without ctx?
2 participants
If the sandbox enters killing state and the operation is canceled or the API crashes, it can cause the sandbox to remain stuck indefinitely.
This prevents getting stuck due to context cancellation + adds a reconcile mechanism