Skip to content

Conversation

@techcodie
Copy link

  • Add clearTimeout() calls in success and error paths
  • Prevents setTimeout references from accumulating
  • Fixes memory leak in high-throughput applications
  • Add test to verify timeout cleanup behavior

Fixes memory leak where buffered collection operations would create setTimeout timers that were never cleared when operations completed successfully, causing continuous memory growth.

Summary

Examples

- Add clearTimeout() calls in success and error paths
- Prevents setTimeout references from accumulating
- Fixes memory leak in high-throughput applications
- Add test to verify timeout cleanup behavior

Fixes memory leak where buffered collection operations would
create setTimeout timers that were never cleared when operations
completed successfully, causing continuous memory growth.
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

clearTimeout looks good but there's some unrelated changes that are incorrect

Keep only the setTimeout memory leak fix as requested by maintainers.
Revert changes to connection.js that were unrelated to the timeout issue.
@techcodie techcodie requested a review from vkarpov15 December 2, 2025 21:03
Move timeout variable declaration to function scope to ensure
it's accessible in all success/error paths for proper cleanup.
- Add clearTimeout() calls in success/error paths to prevent memory leaks
- Move timeout variable to function scope for proper cleanup access
- Revert unrelated otherDbs/relatedDbs changes as requested by maintainer
- Move tests to collection.test.js as requested
@techcodie
Copy link
Author

Hi @vkarpov15! Thank you for the detailed feedback. I've addressed all your comments:

✅ Reverted all unrelated otherDbs/relatedDbs changes in connection.js back to original implementation
✅ Removed separate test/collection.timeout.test.js and moved tests to collection.test.js
✅ PR now contains only the setTimeout memory leak fix with proper clearTimeout() calls

The PR is now focused and clean. I really appreciate your guidance and would love to get this memory leak fix MERGED!

@techcodie techcodie requested a review from vkarpov15 December 3, 2025 03:59
@techcodie
Copy link
Author

@vkarpov15 I have resolved all the issues and rechecked them please let me know if something is wrong or missing I will update it, and if it's not can you merge it please.

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.

2 participants