Skip to content

fix(message): preserve active chat lanes - #1045

Merged
jlucaso1 merged 2 commits into
mainfrom
fix/active-chat-lane-eviction
Jul 16, 2026
Merged

fix(message): preserve active chat lanes#1045
jlucaso1 merged 2 commits into
mainfrom
fix/active-chat-lane-eviction

Conversation

@jlucaso1

Copy link
Copy Markdown
Collaborator

Closes #1039.

An active chat lane could be capacity-evicted after its worker dequeued a message. A later message for the same chat would then create a second worker, allowing post-decryption dispatch to run out of order.

This keeps the cached lane alive from enqueue through processing by carrying a clone of its existing lock with each queued message. The cache now skips lanes with live references, while idle lanes remain evictable on the next capacity check. The token is an Arc clone, so it adds no heap allocation.

The capacity-1 regression test reproduces the old eviction, verifies that consecutive messages stay on the original channel, and confirms the lane becomes evictable after processing.

Performance validation on 50,000-message ping-pong runs:

  • median throughput: 4,747.92 -> 4,751.95 msg/s
  • median client CPU: 3.33 -> 3.30 seconds
  • zero lost or failed messages in all six runs
  • DHAT: no additional allocation blocks; the wider queue slot accounts for 80,352 transient bytes across 10,000 messages (8.04 bytes/message), with nothing retained at exit

Validated with cargo fmt --all --check, cargo clippy --all --tests, the non-E2E workspace test suite, and the release WASM build.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved per-chat message queue handling by enqueueing a richer work item that keeps chat lanes active while messages are in-flight, preventing premature eviction.
    • Ensured capacity pressure retains active lanes so subsequent messages reuse the same processing queue.
    • Preserved existing redelivery behavior when message enqueueing fails.
  • Tests
    • Added an integration test validating chat-lane retention under capacity pressure, reuse of the active worker/channel, and eviction after processing completes.

Walkthrough

Chat-lane queue entries now retain a per-lane guard while processing. Cache eviction checks that guard, message handlers use the new queue payload, and an integration test verifies active lanes survive capacity pressure before becoming evictable when idle.

Changes

Chat lane eviction protection

Layer / File(s) Summary
Queued message contract
src/client.rs
Introduces QueuedChatMessage, containing the node and lane guard, and adds ChatLane::try_enqueue.
Queue and worker integration
src/handlers/message.rs
Routes enqueueing through try_enqueue and updates the chat worker to receive and release composite queue messages.
Eviction guard and capacity validation
src/client/lifecycle.rs, src/client/tests.rs
Evicts lanes only when their enqueue-lock reference count is one, with coverage for active and idle lanes under capacity pressure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: keeping active chat lanes alive.
Description check ✅ Passed The description matches the code changes and explains the lane-preservation fix and test coverage.
Linked Issues check ✅ Passed The PR satisfies #1039 by carrying the lane lock with each message, adding the eviction guard, and covering the capacity-1 case.
Out of Scope Changes check ✅ Passed The changes stay focused on lane retention, eviction logic, and the related regression test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/active-chat-lane-eviction

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a race condition where a capacity-evicted chat_lanes entry could spawn a second worker for the same chat, allowing out-of-order post-decryption dispatch. The fix wraps each queued message in a QueuedChatMessage that carries an Arc clone of enqueue_lock as lane_liveness, and adds an evict_guard to the chat_lanes cache that skips entries with strong_count > 1.

  • QueuedChatMessage bundles the node with a lane_liveness Arc clone; ChatLane::try_enqueue encapsulates creation so callers cannot bypass the guard.
  • The evict_guard in lifecycle.rs mirrors the existing session_locks guard, blocking capacity eviction while any message is queued or in-flight; the lane becomes evictable again only after drop(lane_liveness) runs post-processing.
  • A capacity-1 regression test in tests.rs exercises enqueue, in-flight protection, and post-processing evictability in sequence.

Confidence Score: 5/5

Safe to merge — the fix is a targeted, well-scoped addition that closes a real ordering race without altering any existing hot paths.

The evict_guard pattern is already proven by session_locks; applying it to chat_lanes via an Arc clone carried in QueuedChatMessage is a consistent extension. The strong-count check is race-free within the cache's write lock, explicit drop(lane_liveness) after processing is correctly positioned, and the regression test covers all key scenarios.

No files require special attention.

Important Files Changed

Filename Overview
src/client.rs Introduces QueuedChatMessage (node + lane_liveness Arc) and ChatLane::try_enqueue; cleanly encapsulates strong-count bump at enqueue time.
src/client/lifecycle.rs Adds evict_guard to chat_lanes cache consistent with the existing session_locks guard; one-line change with no other side effects.
src/handlers/message.rs Worker loop destructures QueuedChatMessage with an explicit lane_liveness binding and an explicit drop after processing; intent is anchored by a comment.
src/client/tests.rs Adds active_chat_lane_survives_capacity_pressure regression test covering enqueue, in-flight eviction blocking, and post-processing evictability with a capacity-1 cache.

Reviews (3): Last reviewed commit: "refactor(message): make lane liveness ex..." | Re-trigger Greptile

Comment thread src/handlers/message.rs
Comment thread src/client.rs
Comment thread src/client/tests.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/handlers/message.rs`:
- Around line 88-92: Add a concise comment directly above or beside the
`lane_guard: _lane_guard` binding in the `QueuedChatMessage` destructuring
within the receive loop, documenting that the named binding must remain alive
through the worker body to preserve cache-eviction protection; do not change the
binding behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 88529a47-20c0-4c1b-ae30-7dafcf4e844a

📥 Commits

Reviewing files that changed from the base of the PR and between 0e1f391 and 68e4dca.

📒 Files selected for processing (4)
  • src/client.rs
  • src/client/lifecycle.rs
  • src/client/tests.rs
  • src/handlers/message.rs

Comment thread src/handlers/message.rs
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.93 MiB 10.93 MiB +768 B (+0.01%) 🔺
bin .text 8.91 MiB 8.91 MiB +704 B (+0.01%) 🔺
bin allocated (text+data+bss) 10.93 MiB 10.93 MiB -32 B (-0.00%) 🔽
llvm-lines wacore 514,458 514,458 0
llvm-lines wacore copies 17,564 17,564 0
llvm-lines whatsapp-rust lib 777,560 777,662 +102 (+0.01%) 🔺
llvm-lines whatsapp-rust lib copies 25,297 25,303 +6 (+0.02%) 🔺
deps crates (Cargo.lock) 472 472 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.69 MiB 1.69 MiB +641 B (+0.04%) 🔺
.text wacore 537.22 KiB 537.22 KiB 0
.text wacore_binary 148.45 KiB 148.45 KiB 0
.text wacore_libsignal 203.49 KiB 203.49 KiB 0
.text wacore_appstate 158.31 KiB 158.31 KiB 0
.text wacore_noise 26.03 KiB 26.03 KiB 0
.text waproto 1.60 MiB 1.60 MiB 0
.text whatsapp_rust_sqlite_storage 513.08 KiB 513.08 KiB 0
.text whatsapp_rust_tokio_transport 43.79 KiB 43.79 KiB 0
.text whatsapp_rust_ureq_http_client 10.47 KiB 10.47 KiB 0
.text std 1.02 MiB 1.02 MiB +19 B (+0.00%) 🔺
.text other deps 2.95 MiB 2.95 MiB 0

Baseline: 652d25de5 (latest main run) · Head: 4442470e6 · Graphs

@jlucaso1
jlucaso1 force-pushed the fix/active-chat-lane-eviction branch from 1ec3ba2 to 1a2a20b Compare July 16, 2026 16:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/client.rs`:
- Around line 195-198: Add a Debug derive to the QueuedChatMessage struct while
preserving its existing fields and behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 736042e1-242a-4bda-8a34-2aee162aeeaa

📥 Commits

Reviewing files that changed from the base of the PR and between 1ec3ba2 and 1a2a20b.

📒 Files selected for processing (4)
  • src/client.rs
  • src/client/lifecycle.rs
  • src/client/tests.rs
  • src/handlers/message.rs

Comment thread src/client.rs
@jlucaso1
jlucaso1 merged commit c63d949 into main Jul 16, 2026
22 checks passed
@jlucaso1
jlucaso1 deleted the fix/active-chat-lane-eviction branch July 16, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent active chat lanes from being capacity-evicted

1 participant