-
Notifications
You must be signed in to change notification settings - Fork 0
docs: group SKDM fan-out per-device session locking (whatsapp-rust#990) #388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -677,6 +677,12 @@ Prior to [#807](https://github.com/oxidezap/whatsapp-rust/pull/807), a single ch | |||||||||
| `encrypt_for_devices` is composed of two public halves: `ensure_sessions_for_devices` (network, returns `SessionPlan`) and `encrypt_for_devices_with_sessions` (CPU, consumes `SessionPlan`). The DM path calls `encrypt_for_devices` unchanged; the group path calls them separately with the chain lock taken only around the second. | ||||||||||
| </Note> | ||||||||||
|
|
||||||||||
| <Note> | ||||||||||
| **Per-device session lock around the SKDM fan-out (v0.6).** The chain lock above only serializes the sender-key chain — it does not cover the *pairwise* Signal sessions that `encrypt_for_devices_with_sessions` mutates for each SKDM target device. Those are the same pairwise sessions the DM path locks (see "DM per-device locking" under [Single-allocation session lock keys](#single-allocation-session-lock-keys) below) via `session_lock_for()` / `session_mutexes_for()`. Before [#990](https://github.com/oxidezap/whatsapp-rust/pull/990), the group fan-out held only the chain lock, a disjoint key, so a concurrent DM (or another group send) sharing a device could race that device's pairwise ratchet — both sides load chain index *N* and both store *N+1*, silently dropping one advance. When the lost advance carried the SKDM, that member never received the sender key and every subsequent `skmsg` stayed undecryptable for it until a retry re-distributed. | ||||||||||
|
|
||||||||||
|
Comment on lines
+680
to
+682
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The Note references Prompt for AI agents
Suggested change
|
||||||||||
| `prepare_group_stanza` now acquires the SKDM targets' per-device session locks through `SendContextResolver::lock_device_sessions()` before taking the chain lock, and releases them right after the fan-out — the `skmsg` chain encrypt that follows only touches the sender-key chain, never a pairwise session. The `Client` implementation of this hook reuses `build_session_lock_keys()` + `session_mutexes_for()`, so the group and DM paths serialize on the exact same mutexes, in the same sorted order, and always acquire session locks before the chain lock — no path takes the reverse order, so this cannot deadlock. The hook defaults to a no-op, so a custom `SendContextResolver` (as used in tests and benches) is unaffected unless it opts in. | ||||||||||
| </Note> | ||||||||||
|
|
||||||||||
| ### In-memory sender key device cache | ||||||||||
|
|
||||||||||
| The `SenderKeyDeviceCache` provides an in-memory caching layer over the per-device sender key tracking data stored in the database. Without this cache, every group send would require a database round-trip to load the sender key device map — the cache eliminates that overhead after the first load for each group. | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
session_lock_for()should bebuild_session_lock_keys()The new note mentions
session_lock_for()/session_mutexes_for()as the helpers the DM path uses, butsession_lock_for()does not exist anywhere else in this file or in the PR description. Every other reference in this same file (lines 1402, 1429, 1440) and in the PR description consistently usesbuild_session_lock_keys()+session_mutexes_for(). A developer following this note to look up the implementation would search forsession_lock_for()and find nothing.Prompt To Fix With AI