fix: Fix Memory Leak in Socket Connection Registry on Disconnect

[knoxiboy]

Closes #568

Changes

• Clean up socket registry entries on user disconnect
• Remove stale references to prevent memory growth
• Add proper cleanup in disconnect event handler

Type of Change

• Bug fix

Fixes #568

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved stability of real-time connection handling during rapid reconnections.
  • Enhanced reliability of user online status tracking and message delivery.
  • Optimized database operations to reduce crashes during connection churn.

• Performance

  • Reduced unnecessary database writes for improved system efficiency.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors backend/src/lib/socket.js to fix memory leaks in socket connection registry cleanup, introduce throttled database updates for user presence, and reorganize module structure. It adds proper socket deregistration on disconnect, improves WebRTC call signaling with caller metadata, and prevents listener proliferation through module-scope helper functions.

Changes

Socket Connection Lifecycle and Memory Leak Fix

Layer / File(s)	Summary
Throttled lastSeen cache and rate-limiting backend/src/lib/socket.js	Introduces lastDbUpdateCache object and THROTTLE_MS constant with an async throttledUpdateLastSeen(userId) helper that rate-limits User.findByIdAndUpdate calls to a 30-second window, preventing excessive database writes during connection churn and eliminating undefined-cache crashes on disconnect.
Module-level helpers and handler organization backend/src/lib/socket.js	Relocates canCommunicate(senderId, receiverId) authorization guard to module scope with early rejection for missing or identical IDs; clarifies single io.on("connection") handler enforcement to prevent listener registration growth and ensure proper helper scoping.
Connection registration and initial updates backend/src/lib/socket.js	Registers socket ID in userSocketMap under the user ID, calls throttledUpdateLastSeen to log the connection and cache the update, and initiates pending message delivery workflow when the user comes online.
Real-time typing event handlers with authorization backend/src/lib/socket.js	Refactors typing and stopTyping handlers to gate communication via canCommunicate, resolve all receiver socket IDs from userSocketMap, and emit typing state events to each connected receiver socket.
WebRTC signaling and disconnect cleanup backend/src/lib/socket.js	Enhances callUser to load caller's name for the incomingCall payload; adds callCanceled event handler; unifies WebRTC handlers (answerCall, iceCandidate, endCall, rejectCall) to consistently fetch receiver sockets; rewrites disconnect to remove only the current socket from userSocketMap, delete the user entry when empty, await throttled lastSeen update on full offline transition, evict lastDbUpdateCache, and broadcast the updated online user list.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• UTKARSHH20/fullstack-chat-app#425: Both PRs refactor backend/src/lib/socket.js to introduce/adjust a throttled lastSeen updater backed by an in-memory cache and clear it on final disconnect, directly overlapping on the same connection/disconnect DB-write throttling logic.

Suggested labels

quality:exceptional

Poem

🐰 A socket's journey, now secure and clean,
No leaking pipes in memory's domain supreme!
With throttled writes and helpers set free,
The disconnect cleans up—no stray debris! 🎯

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing a memory leak in the socket connection registry by properly cleaning up on disconnect.
Linked Issues check	✅ Passed	The PR addresses all key objectives from #568: removes stale socket references on disconnect, deletes socket IDs from userSocketMap, dereferences event listeners, and broadcasts offline presence updates.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the memory leak and connection management. The throttled lastSeen helper, connection handler reorganization, and proper disconnect cleanup are all directly related to the issue objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

backend/src/lib/socket.js (1)

86-89: ⚠️ Potential issue | 🟡 Minor

Fix dead/misleading blockedUsers guard in socket logic

backend/src/models/user.model.js defines no blockedUsers field, and the only blockedUsers reference in the repo is this guard in backend/src/lib/socket.js—so the check will never trigger for normal User documents (receiver.blockedUsers is effectively undefined). Either remove this placeholder code/comment or add and persist a real blockedUsers field in the User schema (and ensure it’s populated/available on receiver).

// Example: If your User schema has a 'blockedUsers' array
if (receiver.blockedUsers && receiver.blockedUsers.includes(senderId)) {
    return false;
}

🤖 Prompt for 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.

In `@backend/src/lib/socket.js` around lines 86 - 89, The guard in socket.js
checking receiver.blockedUsers is dead because the User schema has no
blockedUsers; either remove the placeholder check from the message/permission
logic in the function where `receiver` is evaluated (delete the if
(receiver.blockedUsers ... ) block and its comment) or add and persist a real
`blockedUsers` array to the User model (update
`backend/src/models/user.model.js` to include a blockedUsers field and ensure
the code that fetches `receiver` populates that field so
`receiver.blockedUsers.includes(senderId)` works); choose one approach and
update associated tests/fetch logic accordingly so the guard is not misleading.

🤖 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 `@backend/src/lib/socket.js`:
- Around line 195-198: The callCanceled handler lacks the authorization guard;
update the socket.on("callCanceled", async ({ to }) => { ... }) handler to call
canCommunicate(userId, to) (same pattern used in
callUser/answerCall/iceCandidate/endCall/rejectCall) and only proceed to compute
receiverSockets via getReceiverSocketIds(to) and emit
io.to(s).emit("callCanceled") when canCommunicate returns true; if authorization
fails, do not emit and optionally log or ignore the request.

---

Outside diff comments:
In `@backend/src/lib/socket.js`:
- Around line 86-89: The guard in socket.js checking receiver.blockedUsers is
dead because the User schema has no blockedUsers; either remove the placeholder
check from the message/permission logic in the function where `receiver` is
evaluated (delete the if (receiver.blockedUsers ... ) block and its comment) or
add and persist a real `blockedUsers` array to the User model (update
`backend/src/models/user.model.js` to include a blockedUsers field and ensure
the code that fetches `receiver` populates that field so
`receiver.blockedUsers.includes(senderId)` works); choose one approach and
update associated tests/fetch logic accordingly so the guard is not misleading.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 69a772be-4088-4747-bce8-e3f7d9f3a778

📥 Commits

Reviewing files that changed from the base of the PR and between b6ebd50 and bd39a84.

📒 Files selected for processing (1)

• backend/src/lib/socket.js

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing authorization check in callCanceled handler.

All other WebRTC signaling handlers (callUser, answerCall, iceCandidate, endCall, rejectCall) include a canCommunicate(userId, to) check, but callCanceled does not. This allows any authenticated user to send callCanceled to any other user, potentially disrupting their call UI by forcing a clearCall() on the client.

🔒 Proposed fix to add authorization check

 socket.on("callCanceled", async ({ to }) => {
+    if (!(await canCommunicate(userId, to))) return;
     const receiverSockets = getReceiverSocketIds(to);
     receiverSockets.forEach(s => io.to(s).emit("callCanceled"));
 });

🤖 Prompt for 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.

In `@backend/src/lib/socket.js` around lines 195 - 198, The callCanceled handler
lacks the authorization guard; update the socket.on("callCanceled", async ({ to
}) => { ... }) handler to call canCommunicate(userId, to) (same pattern used in
callUser/answerCall/iceCandidate/endCall/rejectCall) and only proceed to compute
receiverSockets via getReceiverSocketIds(to) and emit
io.to(s).emit("callCanceled") when canCommunicate returns true; if authorization
fails, do not emit and optionally log or ignore the request.