Skip to content

fix(webrtc): make session cleanup deadlock-safe - #589

Merged
rustdesk merged 1 commit into
rustdesk:mainfrom
21pages:fix_deadlock
Aug 31, 2026
Merged

fix(webrtc): make session cleanup deadlock-safe#589
rustdesk merged 1 commit into
rustdesk:mainfrom
21pages:fix_deadlock

Conversation

@21pages

@21pages 21pages commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Notes

The issue was identified and fixed by AI, with NO TESTING PERFORMED.

Summary

This PR addresses potential deadlocks during WebRTC session cleanup.

  • Release the data channel lock before awaiting close().
  • Release the session cache lock before closing a duplicate peer.
  • Remove cached sessions by PeerConnection identity instead of SDP fingerprint, preventing delayed callbacks from removing replacement sessions.
  • Add regression tests for session-cache re-entry and identity-based cleanup.

Summary by CodeRabbit

  • Bug Fixes
    • Improved WebRTC session handling to reliably reuse existing connections.
    • Prevented duplicate incoming streams from remaining active.
    • Ensured cached sessions are properly cleaned up when peer connections change state.
    • Reduced the risk of connection hangs during session reuse and closure.

  Release data channel and session cache locks before awaiting close
  operations.

  Remove cached sessions by PeerConnection identity instead of SDP
  fingerprint so delayed callbacks cannot remove replacement sessions.

  Add regression tests for duplicate-session close re-entry and
  identity-based cleanup.

Signed-off-by: 21pages <sunboeasy@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5146633b-54c6-471f-aae9-e351ab29b3b1

📥 Commits

Reviewing files that changed from the base of the PR and between c2d8a6d and 219f917.

📒 Files selected for processing (1)
  • src/webrtc.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The WebRTC session cache now uses dedicated helpers for duplicate-session reuse and peer-specific cleanup. Session creation and terminal peer states use these helpers. Tests cover close behavior without deadlock and selective cleanup.

Changes

WebRTC session cache

Layer / File(s) Summary
Cache helpers and lifecycle integration
src/webrtc.rs
Added helpers to reuse cached sessions and remove sessions that match a peer connection. Updated session creation and terminal peer-connection state cleanup to use the helpers.
Cache behavior tests
src/webrtc.rs
Added test stream setup and tests for duplicate-session reuse without deadlock and cleanup that preserves sessions for unrelated peer connections.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Merge Risk: ⚪ Minimal · up to 219f9

This localized cleanup change releases locks before peer closure and removes sessions by peer identity, reducing deadlock and stale-session deletion risk without changing public WebRTC access or deployment behavior. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making WebRTC session cleanup safe from deadlocks.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@rustdesk
rustdesk merged commit 399030e into rustdesk:main Aug 31, 2026
1 check passed
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.

2 participants