Skip to content

Ensure request cancel is always called#3429

Merged
jakebailey merged 2 commits intomainfrom
jabaile/call-cancel
Apr 16, 2026
Merged

Ensure request cancel is always called#3429
jakebailey merged 2 commits intomainfrom
jabaile/call-cancel

Conversation

@jakebailey
Copy link
Copy Markdown
Member

This appears to be the root cause of #3423. The cancel function from WithCancel, WithTimeout, etc, must be called no matter what. Normally, this is deferred, but this is trickier code because of how we need to order requests and do cancellation out-of-band.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a context leak in the LSP server’s request dispatch loop by ensuring per-request cancellation functions are always invoked when a request is finished/removed, which aligns with Go’s requirement to call cancel funcs from WithCancel/WithTimeout to release resources (relevant to the memory spike reported in #3423).

Changes:

  • Hoist cancel context.CancelFunc so it can be invoked during request cleanup.
  • Ensure cancel() is called when removing a completed request from pendingClientRequests.

@jakebailey jakebailey enabled auto-merge April 16, 2026 19:44
@jakebailey jakebailey requested a review from andrewbranch April 16, 2026 19:49

// Enqueue the debounced diagnostics refresh
s.backgroundQueue.Enqueue(debounceCtx, func(ctx context.Context) {
defer cancel()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code might not call cancelDiagnosticsRefresh and so this could also be dangling. It should be safe to call this here.

@jakebailey jakebailey added this pull request to the merge queue Apr 16, 2026
Merged via the queue into main with commit 7e5b74b Apr 16, 2026
21 checks passed
@jakebailey jakebailey deleted the jabaile/call-cancel branch April 16, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants