Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 36 additions & 19 deletions advanced/signal-protocol.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -765,9 +765,9 @@ Prior to [#807](https://github.com/oxidezap/whatsapp-rust/pull/807), a single ch
</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.
**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_guards_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.

`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.
`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_guards_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>

<Note>
Expand Down Expand Up @@ -1780,9 +1780,9 @@ pub trait JidExt {
}
```

`to_protocol_address_string()` is used on hot paths (message encryption and decryption) as the key for `session_locks`. It pre-sizes the output buffer and builds the string in a single allocation, avoiding the two-allocation overhead of constructing a `ProtocolAddress` and then calling `.to_string()`.
`to_protocol_address_string()` is used on hot paths (message encryption and decryption) as the key for `session_locks`. It pre-sizes the output buffer and builds the `String` in a single allocation. Constructing a `ProtocolAddress` itself no longer allocates for addresses that fit inline (see [Single-buffer ProtocolAddress](#single-buffer-protocoladdress) below), but `.to_string()` on top of it still does, so `to_protocol_address_string()` remains the cheaper path when only the string is needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document the new ProtocolAddress lock keys

This still says the hot-path session_locks key is produced by to_protocol_address_string(), but the inline-SSO refactor uses reusable ProtocolAddress values as the lock-cache keys; otherwise session_guards_for() could not avoid constructing a string key. The same stale model remains in concepts/architecture.mdx, which declares Cache<String, ...>, and in concepts/storage.mdx. Update these descriptions and snippets to the new key type so readers are not given the pre-#1131 allocation and cache behavior.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This premise doesn't hold — I checked against whatsapp-rust@main:

session_locks is still Cache<String, Arc<Mutex<()>>> (src/client.rs:1000), unchanged by #1131. session_lock_for(signal_addr_str: &str) (src/client/adapters.rs:38) calls self.session_locks.get_with_by_ref(signal_addr_str, ...)get_with_by_ref only allocates an owned String key on a cache miss; on a hit it takes &str and allocates nothing. That's what lets session_guards_for() pass addr.as_str() (from a reused ProtocolAddress) without constructing a string key, with no need for the cache itself to be keyed by ProtocolAddress.

This isn't new either — get_with_by_ref already existed at #1131's base commit (1118084), so it predates the inline-SSO refactor entirely. The docs' Cache<String, ...> declaration in concepts/architecture.mdx and the to_protocol_address_string() description in advanced/signal-protocol.mdx are accurate as written. No change needed here.


Generated by Claude Code


The `write_protocol_address_to()` free function provides the same formatting but writes into a caller-supplied `&mut String` buffer, enabling buffer reuse across multiple JIDs (used by `session_mutexes_for()`).
The `write_protocol_address_to()` free function provides the same formatting but writes into a caller-supplied `&mut String` buffer, enabling buffer reuse across multiple JIDs.

**Format examples:**

Expand Down Expand Up @@ -1813,8 +1813,8 @@ let signal_addr_str = encryption_jid.to_protocol_address_string();

// In DM message encryption (src/send.rs) — per-device locks for all devices
let lock_jids = self.build_session_lock_keys(&all_dm_devices).await;
let session_mutexes = self.session_mutexes_for(&lock_jids).await;
// Guards acquired in sorted order to prevent deadlocks
let session_guards = self.session_guards_for(&lock_jids).await;
// Each lock taken as its mutex is resolved, in sorted order, to prevent deadlocks
```

**DM multi-device fanout:**
Expand Down Expand Up @@ -1846,40 +1846,49 @@ To prevent ratchet desync when concurrent sends and receives operate on the same
3. Sorts by `(server, user, device)` using `cmp_for_lock_order()` and deduplicates
4. Returns sorted `Vec<Jid>` — no intermediate `String` allocations needed for sorting

The `session_mutexes_for()` helper then converts sorted JIDs to session mutexes, reusing a single `String` buffer via `write_protocol_address_to()` to avoid per-JID heap allocations:
The `session_guards_for()` helper (PR [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131)) then takes each device's lock as its mutex is resolved from the sorted JIDs, reusing a single `ProtocolAddress` via `reset_protocol_address()` to name each lock key without a formatting buffer:

```rust
// In DM message encryption (src/send.rs)
let lock_jids = self.build_session_lock_keys(&all_dm_devices).await;
let session_mutexes = self.session_mutexes_for(&lock_jids).await;
// Guards acquired in sorted order to prevent deadlocks
let session_guards = self.session_guards_for(&lock_jids).await;
// Each lock taken as its mutex is resolved, in sorted order, to prevent deadlocks
```

<Note>
Resolving every mutex first and then locking each in a second pass (the previous implementation) is still available as `session_mutexes_for()`, but only under `#[cfg(test)]` — production code no longer builds the intermediate `Vec` of mutex handles.
</Note>

The recipient lock key is always the bare form (e.g., `100000012345678@lid.0`), matching the decrypt path's lock format. This ensures send and receive paths serialize on the exact same lock key.

Location: `wacore/src/types/jid.rs:4-51`, `src/send.rs:1481-1507`

### Single-buffer ProtocolAddress

The `ProtocolAddress` struct stores the full address string `"{name}.{device_id}"` in a single `String` buffer, with a `name_len` marker to split name from suffix. This halves the allocation count compared to storing name and device ID separately, and eliminates the copy when rewriting the address via `reset_with()`.
The `ProtocolAddress` struct stores the full address string `"{name}.{device_id}"` in a single `AddressBuf` buffer, with a `name_len` marker to split name from suffix. Since [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131), `AddressBuf` is inline-first: addresses up to 47 bytes (`INLINE_CAPACITY`) live inside the value itself with no heap allocation at all — every real WhatsApp address fits, e.g. `"5511987650001:5@c.us.0"` is 22 bytes. A longer address spills to a `String`.

Which arm holds the bytes is not part of the value: `Eq`, `Ord`, and `Hash` all read the rendered string via `as_str()`, so an inline-built key finds a heap-spilled entry (and vice versa) in the session cache. That equivalence is what makes the optimization safe rather than a silent cache-miss generator.

```rust
// wacore/libsignal/src/core/address.rs
const INLINE_CAPACITY: usize = 47;

pub struct ProtocolAddress {
buf: String, // "{name}.{device_id}" in one buffer
buf: AddressBuf, // inline up to 47 bytes, spills to a `String` beyond that
name_len: usize, // marks where the name ends
device_id: DeviceId,
}

impl ProtocolAddress {
/// One-shot construction: takes ownership of `name` and appends the suffix.
pub fn new(name: String, device_id: DeviceId) -> Self;
/// One-shot construction: writes `name` into the buffer and appends the suffix.
pub fn new(name: &str, device_id: DeviceId) -> Self;

/// Pre-allocated empty address for hot-loop reuse. Call `reset_with()` to fill.
pub fn with_capacity(capacity: usize, device_id: DeviceId) -> Self;
/// An address with no name yet, ready for `reset_with()`. No capacity argument —
/// the buffer starts inline and only allocates if an address ever exceeds it.
pub fn empty(device_id: DeviceId) -> Self;

/// Rewrite the address in place via closure. Single write pass — no intermediate copy.
pub fn reset_with(&mut self, write_name: impl FnOnce(&mut String));
pub fn reset_with(&mut self, write_name: impl FnOnce(&mut AddressBuf));

/// Zero-cost slice of the name portion.
pub fn name(&self) -> &str;
Expand All @@ -1889,7 +1898,15 @@ impl ProtocolAddress {
}
```

Both `name()` and `as_str()` are zero-cost slices into the same buffer — no allocations on access.
Both `name()` and `as_str()` are zero-cost slices into the same buffer — no allocations on access, whether inline or spilled.

<Note>
**API change (PR [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131)):** `ProtocolAddress::new` now takes `name: &str` instead of an owned `String`. `with_capacity(capacity, device_id)` was removed in favor of `empty(device_id)` — the buffer no longer needs a capacity hint, since it starts inline and only allocates on overflow. `reset_with()`'s closure now receives `&mut AddressBuf` instead of `&mut String`; `AddressBuf` implements `push_str`, `push`, and `std::fmt::Write`, which covers existing call sites.
</Note>

<Warning>
`Debug` on `AddressBuf` (and `ProtocolAddress`) must format `as_str()`, never the backing byte array: clearing an inline buffer only rewinds its length, so the unused tail still holds whichever address occupied it before. A derived `Debug` would print the whole array and could leak an unrelated peer's JID into a log line or error that formats a reused address.
</Warning>

### Reusable hot-loop address construction

Expand All @@ -1898,7 +1915,7 @@ When iterating over many devices (e.g., during group stanza preparation or sessi
```rust
// wacore/src/types/jid.rs
pub fn make_reusable_protocol_address() -> ProtocolAddress {
ProtocolAddress::with_capacity(64, SIGNAL_DEVICE_ID)
ProtocolAddress::empty(SIGNAL_DEVICE_ID)
}

pub trait JidExt {
Expand All @@ -1923,7 +1940,7 @@ for device_jid in devices {
}
```

This pattern eliminates one `String` allocation per device in the loop. For a group with 100 participant devices, that saves 100 heap allocations on the send path. The pre-allocated capacity of 64 bytes covers all known WhatsApp address formats without reallocation.
Since [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131), a fresh `ProtocolAddress` for a typical WhatsApp address (up to 47 bytes) is already allocation-free — inline storage covers it. Reuse still matters for addresses that spill to the heap: rewriting a spilled `ProtocolAddress` keeps its existing `String` allocation instead of dropping and reallocating one per device, and for a group with 100 participant devices sharing a reused buffer, that saves up to 100 heap allocations on the send path.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated

<Note>
Use `to_protocol_address()` for one-shot address construction (e.g., cache keys, single lookups). Use `make_reusable_protocol_address()` + `reset_protocol_address()` when iterating over multiple JIDs in a tight loop.
Expand Down
2 changes: 1 addition & 1 deletion api/signal.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ pub async fn create_participant_nodes(
**Returns:**
- `(Vec<Node>, bool)` - The encrypted participant XML nodes and a boolean indicating whether a device identity node should be included in the stanza (true when any participant received a PreKey message).

This method resolves devices, ensures Signal sessions exist, encrypts the message for each device, and returns the resulting XML nodes. It acquires session locks matching the DM send path via `session_mutexes_for()` (bare recipient JID for the recipient, per-device for own companion devices).
This method resolves devices, ensures Signal sessions exist, encrypts the message for each device, and returns the resulting XML nodes. It acquires session locks matching the DM send path via `session_guards_for()` (bare recipient JID for the recipient, per-device for own companion devices) — each lock is taken as its mutex is resolved rather than resolving the whole set first (PR [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131)).

**Example:**
```rust
Expand Down
4 changes: 2 additions & 2 deletions concepts/architecture.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -575,14 +575,14 @@ pub(crate) session_locks: Cache<String, Arc<async_lock::Mutex<()>>>,
// Key examples: "5511999887766@c.us.0", "123456789:33@lid.0"
```

The DM send path resolves all known recipient devices and own companion devices from the local device registry, filters out hosted devices, excludes the sender device, and deduplicates for self-DMs — matching WA Web's `WAWebSendUserMsgJob` and `WAWebDBDeviceListFanout` behavior. The local registry is checked first; a network fetch is only triggered on a cache miss to avoid unnecessary LID-migration side effects. Session locks are acquired for all involved devices in sorted order to prevent deadlocks. The `build_session_lock_keys()` helper resolves encryption JIDs (normalizing the recipient to bare form via `to_non_ad()`), sorts by `(server, user, device)` using `cmp_for_lock_order()`, and deduplicates. The `session_mutexes_for()` helper then converts the sorted JIDs to session mutexes, reusing a single `String` buffer to avoid per-JID heap allocations.
The DM send path resolves all known recipient devices and own companion devices from the local device registry, filters out hosted devices, excludes the sender device, and deduplicates for self-DMs — matching WA Web's `WAWebSendUserMsgJob` and `WAWebDBDeviceListFanout` behavior. The local registry is checked first; a network fetch is only triggered on a cache miss to avoid unnecessary LID-migration side effects. Session locks are acquired for all involved devices in sorted order to prevent deadlocks. The `build_session_lock_keys()` helper resolves encryption JIDs (normalizing the recipient to bare form via `to_non_ad()`), sorts by `(server, user, device)` using `cmp_for_lock_order()`, and deduplicates. The `session_guards_for()` helper then takes each device's lock as its mutex is resolved from the sorted JIDs, rather than resolving the whole set before locking any of them (PR [#1131](https://github.com/oxidezap/whatsapp-rust/pull/1131)); acquisition order is still `jids` order, which is what keeps two overlapping sends from deadlocking.

The peer message path (single-device) acquires a single lock for the resolved encryption JID.

<Note>
**Group SKDM fan-out now shares the DM per-device session locks (v0.6).** `prepare_group_stanza`'s pairwise SKDM fan-out (`encrypt_for_devices_with_sessions`) mutates each target device's pairwise Signal session the same way the DM path does, but it was previously only covered by the per-`(group, sender)` sender-key chain lock — a disjoint key from the DM path's per-device session locks. A concurrent DM (or another group send) sharing a device could therefore race that device's pairwise ratchet: both sides load chain index *N* and both store *N+1*, silently dropping one advance. If the lost advance carried the SKDM, that member never received the sender key and every subsequent `skmsg` was undecryptable for it until a retry re-distributed.

`prepare_group_stanza` now acquires the SKDM targets' per-device session locks — via the new `SendContextResolver::lock_device_sessions()` hook, whose `Client` implementation reuses `build_session_lock_keys()` + `session_mutexes_for()` so both paths serialize on the identical mutexes — before taking the sender-key chain lock, and releases them right after the SKDM fan-out (the `skmsg` chain encrypt that follows only touches the sender-key chain, not any pairwise session). Lock order is always session locks → chain lock on every path, so this cannot introduce a deadlock. The hook defaults to a no-op, so custom `SendContextResolver` implementations (tests, benches) are unaffected unless they opt in.
`prepare_group_stanza` now acquires the SKDM targets' per-device session locks — via the new `SendContextResolver::lock_device_sessions()` hook, whose `Client` implementation reuses `build_session_lock_keys()` + `session_guards_for()` so both paths serialize on the identical mutexes — before taking the sender-key chain lock, and releases them right after the SKDM fan-out (the `skmsg` chain encrypt that follows only touches the sender-key chain, not any pairwise session). Lock order is always session locks → chain lock on every path, so this cannot introduce a deadlock. The hook defaults to a no-op, so custom `SendContextResolver` implementations (tests, benches) are unaffected unless they opt in.
</Note>

Group stanza preparation uses `sort_dedup_by_user()` to deduplicate participants before device resolution, and `sort_dedup_by_device()` to deduplicate resolved device JIDs after LID conversion — both operate in-place on sorted `Vec<Jid>` without `HashSet` allocations.
Expand Down
2 changes: 1 addition & 1 deletion concepts/storage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ struct ByteStoreState {
- **Negative caching**: Known-absent keys are cached as `None` to avoid repeated DB lookups
- **Independent locking**: Sessions, identities, and sender keys each have their own mutex
- **O(1) key cloning**: Keys stored as `Arc<str>` so cloning a key is a refcount bump instead of a heap allocation. The `key_for()` method reuses existing `Arc<str>` keys from the HashMap via `get_key_value()`, avoiding heap allocation on the hot path
- **Single-allocation keys**: Session lock keys use `to_protocol_address_string()` (format: `user[:device]@server.0`) which builds the key string in one allocation, avoiding the two-allocation overhead of constructing a `ProtocolAddress` then calling `.to_string()`. See [Signal Protocol performance](/advanced/signal-protocol#single-allocation-session-lock-keys) for details
- **Single-allocation keys**: Session lock keys use `to_protocol_address_string()` (format: `user[:device]@server.0`) which builds the key string in one allocation. `ProtocolAddress` itself is allocation-free for addresses that fit inline (up to 47 bytes), but `to_string()` on top of it still allocates, so this remains the cheaper path when only the string is needed. See [Signal Protocol performance](/advanced/signal-protocol#single-allocation-session-lock-keys) for details

**Cache operations:**

Expand Down