diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 332c940..e3fe719 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to the KoalaSync browser extension and relay server. --- +## [v2.3.1] — 2026-06-15 + +### Fixed +- **Server: Race condition on concurrent peer joins**: Added a per-peerId serialization lock (`peerJoinLocks`) that prevents two connections with the same `peerId` from both passing the deduplication check and simultaneously registering in `peerToSocket`. Previously, rapid reconnects could leave stale mappings that caused cross-room ACK/PING misdelivery. +- **Server: Crash-safe error handling in teardown paths**: Wrapped `removePeerFromRoom` calls in the disconnect, leave, and reaper handlers with try/catch to prevent an unhandled exception in any teardown path from crashing the process. +- **Server: Smart unhandled rejection handling**: Replaced the immediate `process.exit(1)` on any `unhandledRejection` with a rate-limited approach — the server now exits only after 5 unhandled rejections within 60 seconds, surviving transient errors while still failing fast on cascading crashes. +- **Server: Reduced GC pressure in admin health metrics**: Replaced the three-pass `Array.from()` / `map()` / `reduce()` / `filter()` pattern in `buildHealthPayload()` with a single `for-of` loop, eliminating temporary array allocations proportional to the number of active rooms. +- **Server: Test isolation for rate-limit denial counters**: `stopServerForTests()` now resets the `rateLimitDenied` counters between test runs. + +--- + ## [v2.3.0] — 2026-06-14 ### Added diff --git a/server/index.js b/server/index.js index 4c1650c..42e50f1 100644 --- a/server/index.js +++ b/server/index.js @@ -115,6 +115,7 @@ export const rooms = new Map(); const socketToRoom = new Map(); const peerToSocket = new Map(); // peerId -> socketId (Global lookup) const roomCreationLocks = new Map(); // roomId -> Promise (prevents race on room creation) +const peerJoinLocks = new Map(); // peerId -> Promise (prevents race on same peerId joins) function log(type, message, details = '') { const debugLogging = process.env.DEBUG_LOGGING === '1'; @@ -469,7 +470,20 @@ io.on('connection', (socket) => { return; } - if (!createdByMe) { + let peerLockPromise = peerJoinLocks.get(peerId); + if (peerLockPromise) { + await peerLockPromise; + room = rooms.get(roomId); + if (!room) { + socket.emit(EVENTS.ERROR, { message: "Room no longer exists" }); + return; + } + } + let resolvePeerLock; + peerLockPromise = new Promise(resolve => { resolvePeerLock = resolve; }); + peerJoinLocks.set(peerId, peerLockPromise); + try { + if (!createdByMe) { if (room.passwordHash) { if (!password || hashPassword(password) !== room.passwordHash) { recordAuthFailure(ip, roomId); @@ -527,6 +541,10 @@ io.on('connection', (socket) => { activeLobby: room.activeLobby || null }); log('ROOM', `Peer ${peerId} joined: ${roomId.substring(0, 3)}***`); + } finally { + peerJoinLocks.delete(peerId); + resolvePeerLock(); + } } catch (err) { log('ERROR', `Join error for ${socket.id}`, err); if (socket.connected) { @@ -644,10 +662,14 @@ io.on('connection', (socket) => { }); socket.on(EVENTS.LEAVE_ROOM, () => { - const mapping = socketToRoom.get(socket.id); - if (mapping) { - socket.leave(mapping.roomId); - removePeerFromRoom(socket.id, mapping.roomId, 'leave'); + try { + const mapping = socketToRoom.get(socket.id); + if (mapping) { + socket.leave(mapping.roomId); + removePeerFromRoom(socket.id, mapping.roomId, 'leave'); + } + } catch (err) { + log('ERROR', 'removePeerFromRoom failed in leave', err); } }); @@ -723,10 +745,11 @@ io.on('connection', (socket) => { roomListCooldowns.delete(socket.id); const mapping = socketToRoom.get(socket.id); if (mapping) { - // Socket is already disconnected — no need to call socket.leave(). - // removePeerFromRoom uses io.to() for notifications, which correctly - // excludes this dead socket since it has already left all rooms. - removePeerFromRoom(socket.id, mapping.roomId, 'disconnect'); + try { + removePeerFromRoom(socket.id, mapping.roomId, 'disconnect'); + } catch (err) { + log('ERROR', 'removePeerFromRoom failed in disconnect', err); + } } }); }); @@ -751,12 +774,14 @@ const roomCleanupInterval = setInterval(() => { } } for (const sid of staleSids) { - // Gracefully evict the socket from the Socket.IO room if it is - // still technically connected (zombie with no heartbeat). const deadSocket = io.sockets?.sockets?.get(sid); if (deadSocket) deadSocket.leave(roomId); log('CLEANUP', `Pruning dead peer from room ${roomId.substring(0, 3)}***`); - removePeerFromRoom(sid, roomId, 'reaper'); + try { + removePeerFromRoom(sid, roomId, 'reaper'); + } catch (err) { + log('ERROR', 'removePeerFromRoom failed in reaper', err); + } } // 2. Prune empty or inactive rooms @@ -818,6 +843,7 @@ export async function stopServerForTests() { socketToRoom.clear(); peerToSocket.clear(); roomCreationLocks.clear(); + peerJoinLocks.clear(); connectionCounts.clear(); failedAuthAttempts.clear(); eventCounts.clear(); @@ -827,6 +853,7 @@ export async function stopServerForTests() { healthResponseCache.clear(); io.removeAllListeners(); io.disconnectSockets(true); + Object.assign(rateLimitDenied, { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0 }); if (!httpServer.listening) return; await new Promise((resolve, reject) => { httpServer.close((err) => err ? reject(err) : resolve()); @@ -846,8 +873,19 @@ if (isMainModule) { process.exit(1); }); + let unhandledRejectionCount = 0; + let unhandledRejectionReset = Date.now(); process.on('unhandledRejection', (reason) => { - log('ERROR', `Unhandled rejection: ${reason}`); - process.exit(1); + log('ERROR', `Unhandled rejection: ${reason}`, reason?.stack || ''); + const now = Date.now(); + if (now - unhandledRejectionReset > 60000) { + unhandledRejectionCount = 0; + unhandledRejectionReset = now; + } + unhandledRejectionCount++; + if (unhandledRejectionCount >= 5) { + log('ERROR', `Too many unhandled rejections (${unhandledRejectionCount}/min) — aborting`); + process.exit(1); + } }); } diff --git a/server/ops.js b/server/ops.js index 4445819..d286d3b 100644 --- a/server/ops.js +++ b/server/ops.js @@ -61,19 +61,24 @@ export function buildHealthPayload({ if (!includeMetrics) return payload; - const roomValues = Array.from(rooms.values()); - const roomSizes = roomValues.map(room => room.peers?.size || 0); - const peers = roomSizes.reduce((sum, size) => sum + size, 0); - const maxPeersInRoom = roomSizes.length > 0 ? Math.max(...roomSizes) : 0; - const avgPeersPerRoom = roomSizes.length > 0 - ? Math.round((peers / roomSizes.length) * 100) / 100 + let peers = 0; + let maxPeersInRoom = 0; + let roomsWithLobby = 0; + for (const room of rooms.values()) { + const size = room.peers?.size || 0; + peers += size; + if (size > maxPeersInRoom) maxPeersInRoom = size; + if (room.activeLobby) roomsWithLobby++; + } + const avgPeersPerRoom = rooms.size > 0 + ? Math.round((peers / rooms.size) * 100) / 100 : 0; const mem = memoryUsage(); return { ...payload, peers, - roomsWithLobby: roomValues.filter(room => !!room.activeLobby).length, + roomsWithLobby, avgPeersPerRoom, maxPeersInRoom, rateLimits: {