Skip to content

fix(message-handler): cleanup stale pendingReplies to prevent memory leak#60

Open
kumudasrip wants to merge 5 commits into
snackoverflowasad:mainfrom
kumudasrip:fix/memoryleak
Open

fix(message-handler): cleanup stale pendingReplies to prevent memory leak#60
kumudasrip wants to merge 5 commits into
snackoverflowasad:mainfrom
kumudasrip:fix/memoryleak

Conversation

@kumudasrip

Copy link
Copy Markdown

Description of Changes

  • Added lastActivity tracking for each pending reply entry in src/services/messageHandler.service.ts
  • Implemented periodic cleanup of stale pendingReplies entries after the configured inactivity TTL
  • Added configurable environment variables:
    • CHAT_BUDDY_PENDING_REPLY_TTL_HOURS
    • CHAT_BUDDY_PENDING_REPLY_CLEANUP_INTERVAL_MS
  • Ensured cleanup timer is stopped gracefully on shutdown in src/cli/run.ts
  • Documented the new cleanup settings in README.md

Related Issue Number

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Was This Tested

  • Verified the code changes were applied in src/services/messageHandler.service.ts and src/cli/run.ts
  • Confirmed no editor diagnostics/errors remain in the modified files
  • Added targeted service tests for stale pending reply cleanup and buffered reply processing behavior

Checklist

  • The code builds without errors (npm run build)
  • Existing tests pass (npm test)
  • New tests have been added if needed
  • Documentation has been updated to reflect these changes if needed

@kumudasrip

Copy link
Copy Markdown
Author

Hello @snackoverflowasad @shouri123
I have opened this new PR for the issue #45 , please do review it whenever possible so that I can get to know if the ci build tests are failing again or not, thank you!

@kumudasrip

Copy link
Copy Markdown
Author

I pushed a small follow-up fix after the CI failure.

  • I corrected the remaining local validation issue and reran the focused test.
  • Local verification now passes:
  • npm run build
  • npm test -- --run src/services/messageHandler.service.test.ts --pool=forks

The CI failure looks related to the repo’s formatting/build checks rather than the new pendingReplies cleanup logic itself.

@kumudasrip

Copy link
Copy Markdown
Author

I fixed the follow-up CI failure by removing the duplicate service test file and consolidating the new cleanup tests into the existing tests/messageHandlerService.test.ts suite.
npm test now passes cleanly with 130 tests.

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.

[BUG] Memory leak - pendingReplies Map never cleaned up, grows indefinitely

1 participant