Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 6 additions & 2 deletions src/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,9 +1156,13 @@ impl Client {
// a chain advances past a threshold. Captured-js doesn't show
// the value; 1000 mirrors common Signal hygiene defaults.
const SENDER_KEY_ROTATION_THRESHOLD: u32 = 1000;
// Read the chain iteration through the shared `Arc` without cloning
// the record: borrow the current state instead of `*_mut().cloned()`.
let needs_rotation = record
.and_then(|mut r| r.sender_key_state_mut().ok().cloned())
.and_then(|state| state.sender_chain_key().map(|ck| ck.iteration()))
.as_ref()
.and_then(|r| r.sender_key_state().ok())
.and_then(|state| state.sender_chain_key())
.map(|ck| ck.iteration())
.is_some_and(|iter| iter >= SENDER_KEY_ROTATION_THRESHOLD);
drop(device_guard);

Expand Down
4 changes: 4 additions & 0 deletions src/store/signal_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,14 @@ impl wacore::libsignal::protocol::SenderKeyStore for SenderKeyAdapter {
Option<wacore::libsignal::protocol::SenderKeyRecord>,
> {
let device = self.0.device.read().await;
// group_decrypt mutates the loaded record (catch-up + ratchet) and stores
// it back, so the trait needs an owned copy. The cache keeps its `Arc`, so
// this clones the inner record (unchanged from the prior behavior).
self.0
.cache
.get_sender_key(sender_key_name, &*device.backend)
.await
.map(|opt| opt.map(std::sync::Arc::unwrap_or_clone))
.map_err(signal_err("backend"))
}

Expand Down
41 changes: 37 additions & 4 deletions wacore/src/store/signal_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ impl SessionStoreState {
// === Sender key object cache (same pattern as sessions) ===

struct SenderKeyStoreState {
cache: HashMap<Arc<str>, Option<SenderKeyRecord>>,
// `Arc`-wrapped so a warm `get_sender_key` (the per-send peek reads and the
// per-decrypt load) bumps a refcount instead of deep-cloning the record's
// `VecDeque<SenderKeyState>` with up to `MAX_MESSAGE_KEYS` message keys each.
cache: HashMap<Arc<str>, Option<Arc<SenderKeyRecord>>>,
dirty: HashSet<Arc<str>>,
}

Expand All @@ -163,7 +166,7 @@ impl SenderKeyStoreState {

fn put(&mut self, address: &str, record: SenderKeyRecord) {
let addr = self.key_for(address);
self.cache.insert(addr.clone(), Some(record));
self.cache.insert(addr.clone(), Some(Arc::new(record)));
self.dirty.insert(addr.clone());
}

Expand Down Expand Up @@ -476,18 +479,22 @@ impl SignalStoreCache {

// === Sender Keys ===

/// Returns a shared (`Arc`) handle to the cached sender-key record. A warm hit
/// is a refcount bump, not a deep clone of the message-key backlog. Callers
/// that need to mutate clone the inner record (e.g. via the trait
/// `load_sender_key`), so the cache copy is never mutated through this handle.
pub async fn get_sender_key(
&self,
name: &SenderKeyName,
backend: &dyn SignalStore,
) -> Result<Option<SenderKeyRecord>> {
) -> Result<Option<Arc<SenderKeyRecord>>> {
let key = name.cache_key();
let mut state = self.sender_keys.lock().await;
if let Some(cached) = state.cache.get(key) {
return Ok(cached.clone());
}
let record = match backend.get_sender_key(key).await? {
Some(bytes) => Some(SenderKeyRecord::deserialize(&bytes)?),
Some(bytes) => Some(Arc::new(SenderKeyRecord::deserialize(&bytes)?)),
None => None,
};
state.cache.insert(Arc::from(key), record.clone());
Expand Down Expand Up @@ -680,4 +687,30 @@ mod sender_key_lock_tests {
drop(guard);
assert!(lock.try_lock().is_some(), "released lock must reacquire");
}

#[tokio::test]
async fn warm_sender_key_hit_shares_arc_not_deep_clone() {
let cache = SignalStoreCache::new();
let backend = crate::store::in_memory::InMemoryBackend::new();
let name = SenderKeyName::from_parts("g@g.us", "u@s.whatsapp.net:0");

cache
.put_sender_key(&name, SenderKeyRecord::new_empty())
.await;

let a = cache
.get_sender_key(&name, &backend)
.await
.unwrap()
.expect("warm hit");
let b = cache
.get_sender_key(&name, &backend)
.await
.unwrap()
.expect("warm hit");

// A warm sender-key hit returns a refcount bump of the same allocation,
// not a deep copy of the message-key backlog.
assert!(Arc::ptr_eq(&a, &b));
}
}
Loading