Skip to content

Refactor task execution system: improve call stack management #7035

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

Merged

Conversation

catrielmuller
Copy link

@catrielmuller catrielmuller commented Aug 13, 2025

Related GitHub Issue

Related: #5601

Description

Refactors the recursive recursivelyMakeClineRequests method to use an iterative stack-based approach to prevent potential stack overflow issues and memory leaks.

On some scenarios like a long task continue execution, a deep call stack can cause performance degradation and unnecessary open functions that prevent the run of the GC properly.

Implementation

  • Memory Leak Fix: Enhanced Task.dispose() method with event listener cleanup
  • Added periodic yielding in task processing loops
  • Transformed recursivelyMakeClineRequests() from recursive to iterative implementation
  • Added Nix flake configuration for development environment

Test Procedure

#5601

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Does this PR necessitate updates to user-facing documentation?

  • No documentation updates are required.
  • Yes, documentation updates are required. (Please describe what needs to be updated or link to a PR in the docs

Additional Notes

  • The Nix Flake support it's to enable me to test the extension properly on my end.

Get in Touch

Discord: catrielmuller


Important

Refactor task execution system to use an iterative approach, enhance memory management, and add Nix flake configuration for development.

  • Behavior:
    • Refactor recursivelyMakeClineRequests() in Task.ts from recursive to iterative to prevent stack overflow.
    • Enhance Task.dispose() in Task.ts to remove all event listeners, preventing memory leaks.
    • Add periodic yielding in task processing loops to improve performance.
  • Development Environment:
    • Add Nix flake configuration (flake.nix, flake.lock, .envrc) for development environment setup.
  • Testing:
    • Add Task.dispose.test.ts to test Task.dispose() method for proper resource cleanup and error handling.

This description was created by Ellipsis for a988451. You can customize this summary. It will automatically update as commits are pushed.

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. bug Something isn't working labels Aug 13, 2025
consoleErrorSpy.mockRestore()
})

test("should clean up all resources in correct order", () => {
Copy link

Choose a reason for hiding this comment

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

Consider adding tests to verify that additional cleanup operations (e.g. unsubscribing from task bridge, releasing terminals, closing browsers, and disposing of controllers) are being invoked. This would further ensure that dispose fully prevents resource leaks.

Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I've reviewed the changes and found that while the memory leak fix in the dispose() method is excellent, the recursive-to-iterative refactoring needs some adjustments to fully achieve its goal.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 13, 2025
@daniel-lxs daniel-lxs force-pushed the catrielmuller/improve-task-call-stack branch from a988451 to e60e90b Compare August 14, 2025 16:36
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Aug 14, 2025
Copy link
Collaborator

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

Thank you @catrielmuller!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 14, 2025
@daniel-lxs daniel-lxs force-pushed the catrielmuller/improve-task-call-stack branch from e60e90b to 42f6cf0 Compare August 14, 2025 16:58
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 14, 2025
@mrubens mrubens merged commit 342123d into RooCodeInc:main Aug 14, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 14, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Aug 14, 2025
fxcl added a commit to tameslabs/Roo-Cline that referenced this pull request Aug 16, 2025
* main: (70 commits)
  fix: use native Ollama API instead of OpenAI compatibility layer (RooCodeInc#7137)
  feat: add support for OpenAI gpt-5-chat-latest model (RooCodeInc#7058)
  Make enhance with task history default to true (RooCodeInc#7140)
  Bump cloud version to 0.16.0 (RooCodeInc#7135)
  Release: v1.51.0 (RooCodeInc#7130)
  Add an API for resuming tasks by ID (RooCodeInc#7122)
  Add support for task page event population (RooCodeInc#7117)
  fix: add type check before calling .match() on diffItem.content (RooCodeInc#6905) (RooCodeInc#6906)
  Fix: Enable save button for provider dropdown and checkbox changes (RooCodeInc#7113)
  fix: Use cline.cwd as primary source for workspace path in codebaseSearchTool (RooCodeInc#6902)
  Hotfix multiple folder workspace checkpoint (RooCodeInc#6903)
  fix: prevent XML entity decoding in diff tools (RooCodeInc#7107) (RooCodeInc#7108)
  Refactor task execution system: improve call stack management (RooCodeInc#7035)
  Changeset version bump (RooCodeInc#7104)
  feat(web): fill missing SEO-related values (RooCodeInc#7096)
  Update contributors list (RooCodeInc#6883)
  Release v3.25.15 (RooCodeInc#7103)
  fix: add /evals page to sitemap generation (RooCodeInc#7102)
  feat: implement sitemap generation in TypeScript and remove XML file (RooCodeInc#6206)
  fix: reset condensing state when switching tasks (RooCodeInc#6922)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants