fix the RPC race condition

[xianshijing-lk]

This PR fixes this problem:

In perform_rpc (lines 802-822):

match self
.publish_rpc_request(RpcRequest { ... })
.await // ← Network request sent HERE
{
Ok(_) => {
let mut rpc_state = self.local.rpc_state.lock();
rpc_state.pending_acks.insert(id.clone(), ack_tx); // ← Registration AFTER
rpc_state.pending_responses.insert(id.clone(), response_tx);
}
...
}

The sequence is:

1. publish_rpc_request() sends data over the network via WebRTC DataChannel
2. The .await returns when the data is handed off to the transport
3. Then the code inserts the oneshot channels into pending_acks and pending_responses

Summary by CodeRabbit

• Bug Fixes
  • Improved RPC request handling to prevent race conditions between request publication and response handler registration.
  • Enhanced error cleanup to reliably remove stale pending requests when publish failures occur, ensuring consistent internal state management.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request reorganizes RPC request handling in local_participant.rs to register response and ACK handlers before publishing requests, improving error handling cleanup to prevent stale entries in RPC state.

Changes

Cohort / File(s)	Summary
RPC Request/Response Flow livekit/src/room/participant/local_participant.rs	Reorganized RPC handling to register response/ACK channels before publishing requests, added immediate cleanup of pending entries on publish failure, and refined control flow for consistent state management.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Before the message takes its flight,
Now handlers wait with all their might,
The channels ready, poised to catch,
Each response with a perfect match,
No stale whispers left behind,
A cleaner RPC, well-designed! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix the RPC race condition' clearly and concisely describes the main change in the pull request, which is fixing a race condition in RPC handling by reordering channel registration.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sxian/CLT-2524/fix_rpc_race_condition

---

🧹 Recent nitpick comments

livekit/src/room/participant/local_participant.rs (1)

836-840: Consider explicitly handling the sender-dropped case for robustness.

The Ok(_) pattern matches both successful ack receipt (Ok(Ok(()))) and the case where the sender was dropped without sending (Ok(Err(RecvError))). While the current code paths make the latter unlikely, explicitly handling it would make the intent clearer and guard against future changes.

♻️ Suggested refinement

-            Ok(_) => {
-                // Ack received, continue to wait for response
-            }
+            Ok(Ok(())) => {
+                // Ack received, continue to wait for response
+            }
+            Ok(Err(_)) => {
+                // Sender dropped unexpectedly (shouldn't happen in normal flow)
+                let mut rpc_state = self.local.rpc_state.lock();
+                rpc_state.pending_responses.remove(&id);
+                return Err(RpcError::built_in(RpcErrorCode::ConnectionTimeout, None));
+            }

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7424141 and b83e919.

📒 Files selected for processing (1)

• livekit/src/room/participant/local_participant.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

• GitHub Check: Build (armv7-linux-androideabi)
• GitHub Check: Build (aarch64-apple-darwin)
• GitHub Check: Build (x86_64-linux-android)
• GitHub Check: Build (x86_64-unknown-linux-gnu)
• GitHub Check: Build (aarch64-apple-ios)
• GitHub Check: Build (aarch64-apple-ios-sim)
• GitHub Check: Build (aarch64-linux-android)
• GitHub Check: Build (x86_64-pc-windows-msvc)
• GitHub Check: Build (x86_64-apple-darwin)
• GitHub Check: Build (aarch64-unknown-linux-gnu)
• GitHub Check: Build (aarch64-pc-windows-msvc)
• GitHub Check: Test (x86_64-pc-windows-msvc)
• GitHub Check: Test (x86_64-unknown-linux-gnu)
• GitHub Check: Test (x86_64-apple-darwin)

🔇 Additional comments (1)

livekit/src/room/participant/local_participant.rs (1)

802-827: LGTM! The race condition fix is correct and well-structured.

The reordering properly ensures handlers are registered before the RPC request is published over the network, eliminating the window where responses/acks could arrive before registration. Key improvements:

1. Channel registration now happens atomically in a scoped lock before any network I/O.
2. Cleanup on publish failure correctly removes both pending_acks and pending_responses entries to prevent leaks.
3. Lock is properly scoped and not held across await points.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ladvoc]

LGTM ✅