From 1e37448e05e12aea9348405313ff62cddb0255d4 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Sun, 26 Apr 2026 14:14:16 -0300 Subject: [PATCH] fix(inbound): avoid messageMutex deadlock in appStateSyncKeyShare branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review on PR #392. The previous commit wrapped postUpsertWork in messageMutex.mutex(chatId, ...) for both branches and then awaited it in the appStateSyncKeyShare path. This deadlocks when upsertMessage is invoked from messages-recv.ts:2416, which is already holding messageMutex.mutex(chatId, ...) for the same chat: outer mutex (in caller) → await upsertMessage(...) └─ upsertMessage work() → schedules inner mutex on same chatId (queued) └─ await postUpsertWork ← inner cannot acquire while outer holds it outer cannot release while it awaits inner = eternal deadlock (async-mutex is not re-entrant) Fire-and-forget path is safe because it does NOT await: outer releases as soon as upsertMessage returns, then the queued inner acquires. Fix: in the appStateSyncKeyShare branch, run postUpsertTasks() inline (no inner messageMutex). Per-chat ordering is still preserved by the caller's outer mutex while we await. Other paths keep the inner mutex wrapper for the latency win + per-chat ordering of detached work. --- src/Socket/chats.ts | 51 ++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/Socket/chats.ts b/src/Socket/chats.ts index 8673346a..e0ec7af4 100644 --- a/src/Socket/chats.ts +++ b/src/Socket/chats.ts @@ -1411,20 +1411,12 @@ export const makeChatsSocket = (config: SocketConfig) => { } } - // Fire-and-forget: post-upsert work (history app-state sync + processMessage) - // must NOT block the buffered function. Awaiting here keeps `messages.upsert` - // pinned in the event buffer (createBufferedFunction only schedules flush - // after work() resolves), delaying delivery to the consumer by the duration - // of processMessage. - // - // We wrap the work in `messageMutex.mutex(chatId, ...)` so per-chat ordering - // of processMessage side-effects (chat.unreadCount, LID/PN mapping, messages.update, - // history downloads) is preserved across messages of the same chat. The outer - // mutex acquired by messages-recv.ts releases as soon as upsertMessage returns - // (fast, since work is detached), then this inner mutex enqueues per-chat — - // no deadlock, strict per-chat ordering kept. - const postUpsertChatId = msg.key.remoteJid || msg.key.id || 'unknown' - const postUpsertWork = messageMutex.mutex(postUpsertChatId, () => + // Post-upsert work: history app-state sync + processMessage side effects. + // Awaiting here keeps `messages.upsert` pinned in the event buffer + // (createBufferedFunction only schedules flush after work() resolves), so + // the hot path detaches this work to release the emit on the next debounce + // tick. + const postUpsertTasks = () => Promise.all([ shouldProcessHistoryMsg ? doAppStateSync() : Promise.resolve(), processMessage(msg, { @@ -1439,16 +1431,24 @@ export const makeChatsSocket = (config: SocketConfig) => { getMessage }) ]) - ) - // appStateSyncKeyShare path: processMessage persists the new app-state-sync key - // via keyStore.transaction. The follow-up doAppStateSync() needs that key to - // decrypt patches, so for this rare branch we MUST await postUpsertWork first. - // All other messages stay fire-and-forget (the latency win). - if (msg.message?.protocolMessage?.appStateSyncKeyShare && syncState === SyncState.Syncing) { + const isKeyShareDuringSync = + !!msg.message?.protocolMessage?.appStateSyncKeyShare && syncState === SyncState.Syncing + + if (isKeyShareDuringSync) { + // appStateSyncKeyShare path: processMessage persists the new app-state-sync + // key via keyStore.transaction at process-message.ts:650. The follow-up + // doAppStateSync() needs that key to decrypt patches, so we MUST await + // processMessage first. + // + // IMPORTANT: do NOT wrap in messageMutex here. The inbound caller in + // messages-recv.ts:2416 already holds messageMutex.mutex(chatId, ...) for + // this chat; re-acquiring the same keyed mutex while awaiting it would + // deadlock (async-mutex is not re-entrant). Running inline preserves + // per-chat ordering via the caller's outer mutex. logger.info('App state sync key arrived, awaiting persistence before triggering sync') try { - await postUpsertWork + await postUpsertTasks() await doAppStateSync() } catch (err) { logger?.warn( @@ -1457,7 +1457,14 @@ export const makeChatsSocket = (config: SocketConfig) => { ) } } else { - postUpsertWork.catch(err => + // Fire-and-forget path: wrap in inner messageMutex(chatId) so per-chat + // ordering of processMessage side effects (chat.unreadCount, LID/PN mapping, + // messages.update, history downloads) is preserved across messages of the + // same chat. Reentrancy is safe here because the outer mutex acquired by + // messages-recv.ts releases as soon as upsertMessage returns (work is not + // awaited), then this inner mutex enqueues per-chat. + const postUpsertChatId = msg.key.remoteJid || msg.key.id || 'unknown' + messageMutex.mutex(postUpsertChatId, postUpsertTasks).catch(err => logger?.warn( { err,