Skip to content
Closed
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
28 changes: 14 additions & 14 deletions src/cache_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,28 +159,28 @@ impl CacheStores {
pub struct CacheConfig {
/// Group metadata cache (time_to_live). Default: 1h TTL, 250 entries.
pub group_cache: CacheEntryConfig,
/// Device registry cache (time_to_live). Default: 1h TTL, 5000 entries.
/// Device registry cache (time_to_live). Default: 1h TTL, 1000 entries.
pub device_registry_cache: CacheEntryConfig,
/// LID-to-phone cache (time_to_idle). Default: 1h timeout, 10000 entries.
/// LID-to-phone cache (time_to_idle). Default: 1h timeout, 2000 entries.
pub lid_pn_cache: CacheEntryConfig,
/// Retried group messages tracker (time_to_live). Default: 5m TTL, 2000 entries.
/// Retried group messages tracker (time_to_live). Default: 5m TTL, 500 entries.
pub retried_group_messages: CacheEntryConfig,
/// Optional L1 in-memory cache for sent messages (retry support).
/// Default: capacity 0 (disabled — DB-only, matching WA Web).
/// Set capacity > 0 to enable a fast in-memory cache in front of the DB.
pub recent_messages: CacheEntryConfig,
/// Message retry counts (time_to_live). Default: 5m TTL, 1000 entries.
/// Message retry counts (time_to_live). Default: 5m TTL, 500 entries.
pub message_retry_counts: CacheEntryConfig,
/// PDO pending requests (time_to_live). Default: 30s TTL, 500 entries.
/// PDO pending requests (time_to_live). Default: 30s TTL, 200 entries.
pub pdo_pending_requests: CacheEntryConfig,
Comment on lines +174 to 175

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

pdo_pending_requests default appears out of sync with runtime capacity.

Line 247 changes default capacity to 200, but src/pdo.rs (Line 39 in the provided snippet) still uses a hardcoded .max_capacity(500). That makes this config value ineffective for the actual PDO cache path.

Suggested follow-up patch (outside this file)
-// src/pdo.rs
-.max_capacity(500)
+// src/pdo.rs
+.max_capacity(cache_config.pdo_pending_requests.capacity.max(1))

Also applies to: 247-247

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cache_config.rs` around lines 174 - 175, The config field
pdo_pending_requests (type CacheEntryConfig) has its default capacity changed to
200 but the PDO cache instantiation still hardcodes .max_capacity(500) in
src/pdo.rs; update the PDO cache creation to read the capacity from the
pdo_pending_requests config instead of using .max_capacity(500) (e.g., use the
CacheEntryConfig.capacity/ max_capacity accessor on the pdo_pending_requests
instance when calling the cache builder), or alternatively align the default
back to 500 so the hardcoded value and config match; locate the cache
construction code in pdo.rs that calls .max_capacity(500) and replace it to use
the pdo_pending_requests configuration value.

/// Sender key device tracking cache (time_to_idle). Default: 1h TTI, 500 entries.
/// Caches per-group SKDM distribution state to avoid DB reads on every group send.
pub sender_key_devices_cache: CacheEntryConfig,

// --- Coordination caches (capacity-only, no TTL) ---
/// Per-device Signal session lock capacity. Default: 10000.
/// Per-device Signal session lock capacity. Default: 2000.
pub session_locks_capacity: u64,
/// Per-chat lane capacity (combined lock + queue). Default: 5000.
/// Per-chat lane capacity (combined lock + queue). Default: 1000.
pub chat_lanes_capacity: u64,

// --- Sent message DB cleanup ---
Expand Down Expand Up @@ -239,18 +239,18 @@ impl Default for CacheConfig {

Self {
group_cache: CacheEntryConfig::new(one_hour, 250),
device_registry_cache: CacheEntryConfig::new(one_hour, 5_000),
lid_pn_cache: CacheEntryConfig::new(one_hour, 10_000),
retried_group_messages: CacheEntryConfig::new(five_min, 2_000),
device_registry_cache: CacheEntryConfig::new(one_hour, 1_000),
lid_pn_cache: CacheEntryConfig::new(one_hour, 2_000),
retried_group_messages: CacheEntryConfig::new(five_min, 500),
recent_messages: CacheEntryConfig::new(five_min, 0),
message_retry_counts: CacheEntryConfig::new(five_min, 1_000),
pdo_pending_requests: CacheEntryConfig::new(Some(Duration::from_secs(30)), 500),
message_retry_counts: CacheEntryConfig::new(five_min, 500),
pdo_pending_requests: CacheEntryConfig::new(Some(Duration::from_secs(30)), 200),
sender_key_devices_cache: CacheEntryConfig::new(one_hour, 500),
// Coordination caches hold live mutexes/senders; capacity eviction
// while a reference is held creates a second lock for the same key,
// breaking serialization. Size generously to avoid eviction pressure.
session_locks_capacity: 10_000,
chat_lanes_capacity: 5_000,
session_locks_capacity: 2_000,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Increase session lock capacity to prevent duplicate mutexes

Lowering session_locks_capacity to 2,000 makes capacity eviction much more likely in long-lived/high-fanout sessions, and this cache stores live per-address mutexes used by Client::session_lock_for to serialize Signal session access. Once a key is evicted while an old Arc<Mutex<()>> is still held, the next lookup creates a second mutex for the same address, so concurrent encrypt/decrypt paths can run without mutual exclusion and race session state updates. The adjacent comment already documents that eviction here breaks serialization, so this default reduction introduces a real correctness risk rather than just a memory tradeoff.

Useful? React with 👍 / 👎.

chat_lanes_capacity: 1_000,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Raise chat lane capacity to prevent duplicate per-chat workers

chat_lanes is a capacity-evicted coordination cache, and this default (1_000) is now low enough to hit in long-lived/offline-sync sessions with many distinct chats. When a lane is evicted while its old worker is still draining queued messages (MessageHandler::handle + create_chat_lane in src/handlers/message.rs), the next message for that chat creates a new lane/worker, so two workers process the same chat concurrently and message order guarantees can be violated. The comment immediately above this setting already documents that eviction can break serialization, so lowering this default introduces correctness risk rather than only a memory tradeoff.

Useful? React with 👍 / 👎.

sent_message_ttl_secs: 300,
cache_stores: CacheStores::default(),
}
Expand Down
4 changes: 2 additions & 2 deletions src/socket/noise_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl NoiseSocket {

// Create channel for send jobs. Buffer size of 32 allows multiple
// callers to enqueue work without blocking on channel capacity.
let (send_job_tx, send_job_rx) = async_channel::bounded::<SendJob>(32);
let (send_job_tx, send_job_rx) = async_channel::bounded::<SendJob>(8);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines +47 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid hardcoded latency claims in the queue-capacity comment.

Line 48 says jobs complete in “<1ms each,” but this path includes transport.send(...).await, so latency is network-dependent and not guaranteed. Keep the comment rationale-focused (memory/backpressure) instead of fixed timing.

Suggested patch
-        // Create channel for send jobs. Buffer of 8 is sufficient since the
-        // sender task processes jobs serially in <1ms each.
+        // Keep this queue small to cap pre-allocation and apply backpressure
+        // during bursts; default tuned for typical single-client usage.
As per coding guidelines, "Keep code comments focused on the 'why', not the 'what'; avoid verbose explanations".
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Create channel for send jobs. Buffer of 8 is sufficient since the
// sender task processes jobs serially in <1ms each.
let (send_job_tx, send_job_rx) = async_channel::bounded::<SendJob>(8);
// Keep this queue small to cap pre-allocation and apply backpressure
// during bursts; default tuned for typical single-client usage.
let (send_job_tx, send_job_rx) = async_channel::bounded::<SendJob>(8);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/socket/noise_socket.rs` around lines 47 - 49, The comment above the
async_channel::bounded::<SendJob>(8) creation contains a hardcoded latency claim
("<1ms each") which is incorrect because transport.send(...).await introduces
network-dependent latency; update the comment for send_job_tx/send_job_rx to
remove the timing assertion and instead explain the rationale for capacity=8 in
terms of memory usage and backpressure (e.g., small fixed buffer to limit
in-memory queued SendJob items and to provide modest buffering for the serial
sender), keeping the explanation focused on "why" not timing guarantees.


// Spawn the dedicated sender task
let transport_clone = transport.clone();
Expand Down Expand Up @@ -78,7 +78,7 @@ impl NoiseSocket {
send_job_rx: async_channel::Receiver<SendJob>,
) {
let mut write_counter: u32 = 0;
let mut enc_buf = Vec::with_capacity(4096);
let mut enc_buf = Vec::with_capacity(1024);
// BytesMut: split().freeze() yields a zero-copy Bytes while retaining
// the underlying allocation for the next frame.
let mut out_buf = BytesMut::with_capacity(4096);
Expand Down
2 changes: 1 addition & 1 deletion transports/tokio-transport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use wacore::net::{Transport, TransportEvent, TransportFactory, WHATSAPP_WEB_WS_U

pub use tokio_websockets::Connector;

const EVENT_CHANNEL_CAPACITY: usize = 1_024;
const EVENT_CHANNEL_CAPACITY: usize = 64;

static CRYPTO_PROVIDER_INIT: Once = Once::new();

Expand Down
2 changes: 1 addition & 1 deletion wacore/src/store/signal_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn evict_clean_entries<V>(
}

/// Default max entries per store before clean entry eviction triggers.
const DEFAULT_MAX_CACHE_ENTRIES: usize = 10_000;
const DEFAULT_MAX_CACHE_ENTRIES: usize = 2_000;

/// In-memory write-back cache for Signal protocol state.
/// Keys use `Arc<str>` for O(1) clone. Sessions cached as objects (serialized on flush).
Expand Down
Loading