fix: cache native contact lookups so the synchronous getAllContacts() can't block the event loop (#811) - #813
Open
johndkos wants to merge 1 commit into
Open
Conversation
…) off the request hot path ContactsLib.getAllContacts() calls node-mac-contacts' synchronous getAllContacts() on every request. With avatars requested it marshals image data for every contact, which is multi-second on large address books and, being synchronous, blocks the Node event loop — stalling all concurrent requests including the socket health probe and causing clients to drop/reconnect in a loop. The result is now cached (keyed by the requested extra-prop set) and invalidated on the existing contact-changed listener plus a TTL, so repeat fetches never re-run the blocking native call. Fixes BlueBubblesApp#811 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
The only thing I want to be conscious of here is memory usage, especially on older macOS systems. Theoretically, any time the extraProps changes, it could fetch and cache a new, separate version of the contacts list. I'll have to think about this some more. Not a huge issue, but if someone has a huge DB of contacts, then it could be |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Cache
ContactsLib.getAllContacts()results so repeat calls don't re-run the synchronousnode-mac-contactsnative lookup. Fixes #811.Why
ContactsLib.getAllContacts()callsnode-mac-contacts' synchronousgetAllContacts(extraProps)on every request. When images/thumbnails are requested (extraProperties=avatar→contactImage/contactThumbnailImage), it marshals avatar data for every contact — multi-second on large address books. Because the call is synchronous, it blocks the Node event loop for its full duration, stalling every other in-flight HTTP request — including the socket health/reachability probe. Clients (observed: Windows desktop) treat the missed probe as the server being unreachable, drop the socket, reconnect, and re-request contacts-with-avatars — re-triggering the block. The result is a reconnect loop that surfaces as the client stuck on "connecting".The file already had the intent to cache (an
isFirstLoadflag + acontact-changedlistener) but never stored the loaded result, so every call re-paid the full native cost.Measured on a live 1.9.9 server (~2,500 contacts, macOS 15.7.7, Messages 14):
GET /api/v1/contact?extraProperties=avatar≈ 4.3–5.7 s, and aGET /api/v1/pingfired during it was delayed ~4.8 s (vs ~2 ms idle), confirming the event-loop block. Plain/contact(no avatars) and/server/statistics/*were single-digit ms.How
Mapkeyed by the normalized + sorted requested extra-prop set.contact-changedOS listener (immediate) plus a 5-minute TTL safety net for any missed event.A client's reconnect re-fetch now hits the cache and returns instantly, so the probe no longer times out and the loop stops. The first (cold) load still runs once; moving it off-thread (worker) is a possible follow-up, kept out of scope to stay minimal.
Testing
The block was measured on a live 1.9.9 server as described above. The change is localized to
ContactsLiband reuses the existing change-listener. I did not run a full end-to-end build against a production database (to avoid standing up a second server instance alongside a live one) — happy to adjust the TTL or approach per maintainer preference.Related
Sibling of #812 — the statistics endpoints share the same synchronous-on-main-thread + no-cache pattern (
better-sqlite3COUNT(*)).