Skip to content

fix: prevent memory leaks from AbortController in async operations #6126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lkk214
Copy link
Contributor

@lkk214 lkk214 commented Jun 14, 2025

Description

AbortController instances were not being properly cleaned up after async operations (Promise/AsyncGenerator) completed or failed, leading to potential memory leaks.

Testing

Verified with:

  • Run async tasks until completion
  • Premature interruption cases
  • Memory profiling before/after changes

@lkk214 lkk214 requested a review from a team as a code owner June 14, 2025 13:53
@lkk214 lkk214 requested review from sestinj and removed request for a team June 14, 2025 13:53
Copy link

netlify bot commented Jun 14, 2025

👷 Deploy request for continuedev pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 1ffa959

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 14, 2025
Copy link

recurseml bot commented Jun 14, 2025

✨ No issues found! Your code is sparkling clean! ✨

@lkk214 lkk214 force-pushed the fix/abort-controller-memory-leak branch from d7bf38e to fed3049 Compare June 14, 2025 14:29
Copy link
Contributor

@sestinj sestinj left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, it's a great fix. I do think it would be good to have a quick test for the runWithAbortController method, since this is going to be on the critical path of every single request. Once there are tests I'm happy to merge!

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs Jun 16, 2025
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 21, 2025
@lkk214 lkk214 marked this pull request as draft June 21, 2025 15:40
@lkk214
Copy link
Contributor Author

lkk214 commented Jun 21, 2025

Since #5828 still uses the original implementation of addMessageAbortController and abortById, I’ve marked this PR as draft for now. Will update the references here once #5828 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants