Skip to content

fix: prevent memory leak from orphaned sockets on reconnect - #19

Open
FranzFelberer wants to merge 2 commits into
jlucaso1:mainfrom
FranzFelberer:fix/reconnect-memory-leak
Open

fix: prevent memory leak from orphaned sockets on reconnect#19
FranzFelberer wants to merge 2 commits into
jlucaso1:mainfrom
FranzFelberer:fix/reconnect-memory-leak

Conversation

@FranzFelberer

@FranzFelberer FranzFelberer commented Jun 10, 2026

Copy link
Copy Markdown

Problem

On every non-logout disconnect, startWhatsAppConnection() recursively calls itself (src/whatsapp.ts):

if (statusCode !== DisconnectReason.loggedOut) {
  logger.info("Reconnecting...");
  startWhatsAppConnection(logger); // new socket; old one is never ended
}

This builds a brand-new socket without ending the previous one or detaching its ev.process handler. Under a WhatsApp-side disconnect storm (timedOut close events — I've observed bursts of thousands per minute), orphaned sockets pile up, each retaining its own WebSocket, keep-alive timer and signal-key cache. Memory grows unbounded — in my setup the process reached ~1.8 GB after ~10 days (peak ~2.9 GB).

There is also a latent correctness bug: the recursive call returns a new socket, but the caller (the MCP server) keeps the original reference — so after the first reconnect, sendMessage targets a dead socket.

Fix

Rework the connection into a single self-healing loop:

  • Teardown the dying socket before reconnecting — detach the ev.process handler (via the cleanup function it returns) and call sock.end() — so its resources become collectable.
  • Single-flight guard + exponential backoff (1s → 30s, reset on open), so a disconnect storm can no longer spin up sockets unbounded.
  • Return a stable Proxy that always delegates to the live socket, so callers keep a valid reference across reconnects.

The public API (startWhatsAppConnection, sendWhatsAppMessage) and behaviour are unchanged.

Result

In production, a bridge that had grown to ~1.8 GB dropped to ~80–130 MB and stayed flat across reconnects.

One file changed (src/whatsapp.ts); node --check clean.

Summary by CodeRabbit

  • Bug Fixes & Improvements
    • Enhanced WhatsApp connection stability with automatic reconnection and self-healing behavior
    • Improved handling of disconnects with exponential backoff and single-flight reconnection guarding
    • Prevented stale connection references so callers keep a live, transparent connection handle
    • Clearer separation of recoverable vs non-recoverable connection errors and safer cleanup

On every non-logout disconnect, startWhatsAppConnection() recursively called
itself, creating a new socket without ending the previous one or detaching its
ev.process handler. Under a WhatsApp-side disconnect storm (timedOut closes -- I
have seen bursts of thousands per minute in production), orphaned sockets
accumulate, each retaining its WebSocket, keep-alive timer and signal-key cache,
leaking memory until the process reaches several GB.

Latent second bug: the recursive call returns a new socket while the caller (the
MCP server) keeps the original reference, so sends target a dead socket after
the first reconnect.

Rework the connection into a single self-healing loop:
- tear down the dying socket (detach the ev handler + sock.end()) before
  reconnecting, so its resources become collectable
- single-flight guard + exponential backoff (1s..30s), reset on "open", so a
  storm can no longer spin up sockets unbounded
- return a stable Proxy that always delegates to the live socket, so callers
  keep a valid reference across reconnects

Behaviour is otherwise unchanged. In production this dropped a bridge that had
grown to ~1.8 GB (10 days uptime) to ~80-130 MB.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba6c0b59-dcbf-4a06-b859-3f741a4bdb55

📥 Commits

Reviewing files that changed from the base of the PR and between cb5de0a and f136739.

📒 Files selected for processing (1)
  • src/whatsapp.ts

📝 Walkthrough

Walkthrough

startWhatsAppConnection() is refactored to maintain a self-healing WhatsApp socket using explicit state tracking, exponential-backoff reconnection scheduling, and a stable Proxy interface. The function now creates sockets inside a connect() function, tears down disconnected sockets completely, schedules reconnects instead of recursively restarting, and returns a Proxy to prevent callers from holding stale references.

Changes

WhatsApp Connection Self-Healing Infrastructure

Layer / File(s) Summary
API doc for stable connection handle
src/whatsapp.ts
Adds documentation clarifying that startWhatsAppConnection() returns a stable Proxy handle that delegates to the live socket and transparently survives reconnects.
Reconnection infrastructure and state tracking
src/whatsapp.ts
Introduces currentSock state, detachable event-handler cleanup, single-flight reconnecting, exponential-backoff attempts, explicit teardown(), scheduleReconnect(), and a connect() that creates the socket and updates state.
Connection event handlers and state transitions
src/whatsapp.ts
Updates connection.update handling to schedule reconnects for non-logged-out closes, exit on logged-out closes, and reset backoff attempts on connection === "open".
Stable socket proxy interface
src/whatsapp.ts
Returns a Proxy instead of the raw socket; the proxy forwards property access, assignments, and method calls to currentSock and preserves a stable handle across reconnects.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I stitched a socket that mends through the night,
Hops back on line with backoff in sight,
Callers hold a proxy, steady and kind,
No stale handlers left trailing behind,
🥕 Reconnects hop in—robust and polite.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: prevent memory leak from orphaned sockets on reconnect' directly and specifically describes the main problem being solved: preventing memory leaks caused by orphaned sockets during reconnection.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/whatsapp.ts`:
- Around line 126-147: The teardown function leaves currentSock pointing to the
closed socket so callers can still use the dead connection; modify teardown (the
teardown(dead: WhatsAppSocket) function) to clear the live-socket pointer (set
currentSock = null or equivalent) before ending the socket, ensuring the proxy
no longer exposes the dead instance during backoff; keep existing detach
handling and dead.end call and update any related state (if any) used by
scheduleReconnect/connect so reconnect logic remains unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cc0a2f53-05d7-4027-bfae-12c7321b0010

📥 Commits

Reviewing files that changed from the base of the PR and between ce9b144 and cb5de0a.

📒 Files selected for processing (1)
  • src/whatsapp.ts

Comment thread src/whatsapp.ts
Addresses the CodeRabbit review on jlucaso1#19: during the reconnect backoff window,
teardown() ended the socket but left currentSock pointing at it, so the Proxy
still exposed the dead connection and a send attempted in that window targeted
an already-ended socket.

- teardown() now nulls currentSock (guarded by identity) before ending it
- the Proxy reports undefined while disconnected, so sendWhatsAppMessage's
  `!sock.user` guard cleanly reports "not connected" during backoff
- guard the Proxy set trap too, for symmetry
- add a JSDoc to startWhatsAppConnection

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant