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
2 changes: 1 addition & 1 deletion agent_docs/observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ message can overshoot substantially on its own.
| `pdo_pending_requests` / `pdo_requested` | 30s TTL, 200 / 24h TTL, 512 | a repeated PDO request |
| `sender_key_devices_cache` | 1h TTI, 500 | a redundant SKDM |
| `session_recreate_history` | 1h TTL, 256 | one un-throttled recreate |
| `session_locks` / `chat_lanes` / `group_distribution_locks` | 10 000 / 5 000 / 512 | nothing: an `evict_guard` refuses to evict a lock a task holds, so the map briefly exceeds capacity instead of minting a second lock for one key |
| `session_locks` / `chat_lanes` / `group_distribution_locks` | 10 000 / 5 000 / 512 | nothing: an `evict_guard` refuses to evict a lock a task holds, so the map briefly exceeds capacity instead of minting a second lock for one key. A chat lane's worker, which holds the inbound-message future (~9 KiB) for its whole life, exits after `LANE_IDLE_TIMEOUT` (60 s) of silence; the entry then costs one closed channel until the chat's next message replaces it or FIFO eviction drops it |
| `resend_rate_limiter` | 4 096, FIFO | fail-open by design — an evicted bucket is recreated full, so undersizing forgives rate, never over-throttles |
| `group_devices_memo` / `skdm_warm_memo` / `dm_devices_memo` | 64 / 64 / 512 | a recompute |
| `SignalStoreCache` sessions / identities / sender keys | 2 000 each (+1/8 slack before an eviction scan), *while flushes succeed* | nothing: only *clean* entries are evicted, so an unpersisted record is never dropped — which also means a backend that stops accepting writes leaves everything dirty and the maps grow past the cap. Correct, and the reason to watch the counts rather than trust the number |
Expand Down
4 changes: 4 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,10 @@ type ChatStateHandler = Arc<dyn Fn(ChatStateEvent) + Send + Sync>;
pub(crate) struct ChatLane {
pub enqueue_lock: Arc<Mutex<()>>,
pub queue_tx: async_channel::Sender<QueuedChatMessage>,
/// Held by the lane's worker for as long as it runs; a replacement
/// worker takes it before its first message. Why it is shared across
/// lane generations is explained at `create_chat_lane`.
pub worker_running: Arc<Mutex<()>>,
}

impl ChatLane {
Expand Down
10 changes: 10 additions & 0 deletions src/client/lid_pn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,16 @@ impl Client {
phone_number: &str,
source: LearningSource,
) -> RecordOutcome {
// Answered before the mutation mutex: this runs for every message
Comment thread
jlucaso1 marked this conversation as resolved.
// whose sender carries a `sender_alt`, and in the steady state the pair
// is already known both ways and persisted, so the answer is `Skipped`.
// Taking the process-wide mutex first serialized every chat lane on
// the receive path behind a lock that the common case never needed.
// The guarded body re-checks under the lock, so a concurrent write
// still sees a consistent view.
if self.lid_pn_cache.can_skip_relearn(phone_number, lid).await {
return RecordOutcome::Skipped;
}
let guard = self.lid_pn_cache.lock_mutation().await;
self.record_lid_pn_in_memory_guarded(lid, phone_number, source, &guard)
.await
Expand Down
1 change: 1 addition & 0 deletions src/client/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3552,6 +3552,7 @@ async fn active_chat_lane_survives_capacity_pressure() {
ChatLane {
enqueue_lock: Arc::new(Mutex::new(())),
queue_tx,
worker_running: Arc::new(Mutex::new(())),
},
queue_rx,
)
Expand Down
296 changes: 258 additions & 38 deletions src/handlers/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ use wacore::stanza::wire_tags::StanzaTag;
/// WA Web: `WAWebMessageQueue` uses `promiseTimeout(r(), 2e4)` per queued handler.
const MAX_MESSAGE_DELAY_MS: u64 = 20_000;

/// How long a lane worker waits for its next message before exiting.
///
/// A worker awaits `handle_incoming_message_scoped` inline, so its task holds
/// that future's whole state machine (~9 KiB) for as long as the worker lives,
/// message or no message. Kept alive for the connection, that is one such
/// future per chat that ever spoke, bounded only by `chat_lanes_capacity`;
/// a client in a few thousand groups parked tens of MiB in idle workers. An
/// idle worker now exits and the next message for the chat spawns a fresh
/// one, so the cost is one task per burst of activity instead of per chat.
///
/// Long enough that a conversation in progress never respawns between
/// replies; short against the hours a connection stays up.
const LANE_IDLE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60);

/// Handler for `<message>` stanzas.
///
/// Messages are processed sequentially per-chat using a mailbox pattern to prevent
Expand Down Expand Up @@ -40,15 +54,55 @@ impl MessageHandler {
// preventing duplicate workers for the same chat (TOCTOU race).
let lane = client
.chat_lanes
.get_with_by_ref(&chat_jid, async { create_chat_lane(&client) })
.get_with_by_ref(&chat_jid, async {
create_chat_lane(
&client,
Arc::new(async_lock::Mutex::new(())),
Arc::new(async_lock::Mutex::new(())),
)
})
.await;

// Lock serializes enqueue order for this chat
// Lock serializes enqueue order for this chat, the replacement
// below included (see `create_chat_lane` for why it outlives the lane).
let _guard = lane.enqueue_lock.lock().await;

if let Err(e) = lane.try_enqueue(node) {
let node = match lane.try_enqueue(node) {
Ok(()) => return true,
// The worker went idle and closed its queue (see
// `LANE_IDLE_TIMEOUT`). Replace the lane; the successor worker
// starts only once the idle one has finished draining.
Err(async_channel::TrySendError::Closed(queued)) => queued.node,
Err(e) => {
warn!("Failed to enqueue message for processing: {e}");
// Cancel ack so server redelivers
*cancelled = true;
return true;
}
};

// A caller that queued behind this lock on the same stale lane finds
// the replacement already cached and joins it instead of replacing
// it again. Nothing is enqueued before the lane is in the cache, so a
// cancellation here leaves at worst an empty worker that idles out.
let fresh = match client.chat_lanes.get(&chat_jid).await {
Some(current) if !current.queue_tx.same_channel(&lane.queue_tx) => current,
_ => {
client.chat_lanes.invalidate(&chat_jid).await;
client
.chat_lanes
.get_with_by_ref(&chat_jid, async {
create_chat_lane(
&client,
Arc::clone(&lane.enqueue_lock),
Arc::clone(&lane.worker_running),
)
})
.await
}
};
if let Err(e) = fresh.try_enqueue(node) {
warn!("Failed to enqueue message for processing: {e}");
// Cancel ack so server redelivers
*cancelled = true;
}

Expand All @@ -75,57 +129,223 @@ impl StanzaHandler for MessageHandler {

/// Construct a ChatLane with a spawned worker task. Extracted to keep the
/// init closure passed to `get_with_by_ref` small.
fn create_chat_lane(client: &Arc<Client>) -> ChatLane {
///
/// Both locks belong to the chat rather than the lane, which is why they are
/// parameters: a lane that replaces an idle-exited one passes the
/// predecessor's, and only the chat's first lane mints new ones. Sharing
/// `enqueue_lock` keeps every enqueue for the chat in one total order
/// whichever lane generation a handler fetched, and the swap itself runs
/// under it; sharing `worker_running` makes the new worker wait for the old
/// one to finish draining, so the two never process the same chat at once.
fn create_chat_lane(
client: &Arc<Client>,
enqueue_lock: Arc<async_lock::Mutex<()>>,
worker_running: Arc<async_lock::Mutex<()>>,
) -> ChatLane {
let (tx, rx) = async_channel::unbounded::<QueuedChatMessage>();

let client_for_worker = client.clone();
let spawn_generation = client
.connection_generation
.load(std::sync::atomic::Ordering::Acquire);
let running = Arc::clone(&worker_running);

client
.runtime
.spawn(Box::pin(async move {
while let Ok(QueuedChatMessage {
node: msg_node,
lane_liveness, // Prevents capacity eviction until processing finishes.
}) = rx.recv().await
{
if client_for_worker
.connection_generation
.load(std::sync::atomic::Ordering::Acquire)
!= spawn_generation
{
log::debug!(target: "MessageQueue", "Stale worker exiting; remaining messages will be redelivered by server");
// Queue behind the worker this lane replaced, if it is still
// draining the messages that raced its idle exit.
let _running = running.lock_arc().await;
loop {
// A burst is served straight off the queue; the idle timer is
// only armed once the queue is empty, so it costs nothing per
// message while the chat is busy.
let next = match rx.try_recv() {
Ok(queued) => queued,
Err(async_channel::TryRecvError::Closed) => break,
Err(async_channel::TryRecvError::Empty) => {
match wacore::runtime::timeout(
&*client_for_worker.runtime,
LANE_IDLE_TIMEOUT,
rx.recv(),
)
.await
{
Ok(Ok(queued)) => queued,
Ok(Err(_)) => break,
Err(wacore::runtime::Elapsed) => {
// Stop accepting first, then drain whatever
// was enqueued before the close: an enqueue
// that lands after it is told `Closed` and
// replaces this lane.
rx.close();
while let Ok(queued) = rx.try_recv() {
if !process_queued(&client_for_worker, queued, spawn_generation)
.await
{
break;
}
}
break;
}
}
}
};
if !process_queued(&client_for_worker, next, spawn_generation).await {
break;
}
// Two clock reads per message, kept: sampling or gating on lane
// backlog would stop reporting the single pathological message
// this guard exists to catch.
let start = wacore::time::Instant::now();
let client = client_for_worker.clone();
// Awaited inline (not boxed): the future lives in this
// once-per-chat worker task instead of a fresh ~31 KB heap box
// per message, which dominated per-message allocation churn.
client
.handle_incoming_message_scoped(msg_node, spawn_generation)
.await;
let elapsed = start.elapsed();
if elapsed.as_millis() as u64 > MAX_MESSAGE_DELAY_MS {
warn!(
target: "MessageQueue",
"Message processing took {:.1}s (MAX_MESSAGE_DELAY is {}s)",
elapsed.as_secs_f64(),
MAX_MESSAGE_DELAY_MS / 1000
);
}
drop(lane_liveness);
}
}))
.detach();

ChatLane {
enqueue_lock: Arc::new(async_lock::Mutex::new(())),
enqueue_lock,
queue_tx: tx,
worker_running,
}
}

/// Process one queued message on its lane worker. Returns `false` when the
/// worker belongs to a torn-down connection and must stop.
async fn process_queued(
client: &Arc<Client>,
QueuedChatMessage {
node: msg_node,
lane_liveness, // Prevents capacity eviction until processing finishes.
}: QueuedChatMessage,
spawn_generation: u64,
) -> bool {
if client
.connection_generation
.load(std::sync::atomic::Ordering::Acquire)
!= spawn_generation
{
log::debug!(target: "MessageQueue", "Stale worker exiting; remaining messages will be redelivered by server");
return false;
}
// Two clock reads per message, kept: sampling or gating on lane
// backlog would stop reporting the single pathological message
// this guard exists to catch.
let start = wacore::time::Instant::now();
// Awaited inline (not boxed): the future lives in this
// once-per-chat worker task instead of a fresh ~9 KB heap box
// per message, which dominated per-message allocation churn.
Arc::clone(client)
.handle_incoming_message_scoped(msg_node, spawn_generation)
.await;
let elapsed = start.elapsed();
if elapsed.as_millis() as u64 > MAX_MESSAGE_DELAY_MS {
warn!(
target: "MessageQueue",
"Message processing took {:.1}s (MAX_MESSAGE_DELAY is {}s)",
elapsed.as_secs_f64(),
MAX_MESSAGE_DELAY_MS / 1000
);
}
drop(lane_liveness);
true
}

#[cfg(test)]
mod tests {
use super::*;
use crate::test_utils::node_to_owned_ref;
use wacore_binary::builder::NodeBuilder;
use wacore_binary::jid::Jid;

fn message_for(chat: &Jid, id: &str) -> Arc<wacore_binary::OwnedNodeRef> {
node_to_owned_ref(
&NodeBuilder::new("message")
.attr("from", chat.clone())
.attr("id", id)
.build(),
)
}

/// An idle worker closes its queue and exits; the next message for the
/// chat gets a fresh lane whose worker queues behind the old one.
#[tokio::test(start_paused = true)]
async fn an_idle_lane_worker_exits_and_the_next_message_respawns_it() {
let client = crate::test_utils::create_test_client().await;
let chat: Jid = "120363000000000031@g.us".parse().unwrap();

let mut cancelled = false;
assert!(
MessageHandler::handle_inline(
Arc::clone(&client),
message_for(&chat, "A"),
&mut cancelled
)
.await
);
assert!(!cancelled);
let first = client.chat_lanes.get(&chat).await.expect("lane created");
let first_tx = first.queue_tx.clone();
assert!(!first_tx.is_closed());

// Paused time: the sleep advances the clock instead of waiting.
tokio::time::sleep(LANE_IDLE_TIMEOUT + std::time::Duration::from_secs(1)).await;
for _ in 0..100 {
if first_tx.is_closed() {
break;
}
tokio::task::yield_now().await;
}
assert!(first_tx.is_closed(), "idle worker must close its queue");
assert!(
first.worker_running.try_lock().is_some(),
"an exited worker must release its running lock"
);

let mut cancelled = false;
assert!(
MessageHandler::handle_inline(
Arc::clone(&client),
message_for(&chat, "B"),
&mut cancelled
)
.await
);
assert!(
!cancelled,
"a message after the idle exit is accepted, not redelivered"
);
let second = client.chat_lanes.get(&chat).await.expect("lane replaced");
assert!(!second.queue_tx.same_channel(&first_tx));
assert!(!second.queue_tx.is_closed());
assert!(
Arc::ptr_eq(&first.worker_running, &second.worker_running),
"the successor serializes behind the predecessor's running lock"
);
assert!(
Arc::ptr_eq(&first.enqueue_lock, &second.enqueue_lock),
"the chat's enqueue lock survives the swap"
);
}

/// A message enqueued in the window between the idle check and the close
/// is still processed by the exiting worker, and a message after the close
/// is not lost.
#[tokio::test(start_paused = true)]
async fn a_lane_that_stays_busy_never_exits() {
let client = crate::test_utils::create_test_client().await;
let chat: Jid = "120363000000000032@g.us".parse().unwrap();
let mut cancelled = false;
MessageHandler::handle_inline(Arc::clone(&client), message_for(&chat, "A"), &mut cancelled)
.await;
let lane = client.chat_lanes.get(&chat).await.expect("lane created");
for i in 0..5 {
tokio::time::sleep(LANE_IDLE_TIMEOUT / 2).await;
MessageHandler::handle_inline(
Arc::clone(&client),
message_for(&chat, &format!("M{i}")),
&mut cancelled,
)
.await;
assert!(!cancelled);
}
let same = client.chat_lanes.get(&chat).await.expect("lane kept");
assert!(same.queue_tx.same_channel(&lane.queue_tx));
assert!(!lane.queue_tx.is_closed());
}
}
2 changes: 1 addition & 1 deletion src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::sync::Arc;
use std::sync::atomic::{AtomicU8, Ordering};
use wacore::libsignal::crypto::DecryptionError;
use wacore::libsignal::protocol::SenderKeyStore;
use wacore::libsignal::protocol::group_decrypt;
use wacore::libsignal::protocol::group_decrypt_shared;
use wacore::libsignal::protocol::{
CiphertextMessage, DecryptionResult, IdentityChange, OwnedCiphertextMessage,
PreKeySignalMessage, SignalMessage, SignalProtocolError, UsePQRatchet, message_decrypt,
Expand Down
Loading
Loading