fix(webrtc): make session cleanup deadlock-safe - #589
Conversation
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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe 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. ChangesWebRTC session cache
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Notes
The issue was identified and fixed by AI, with NO TESTING PERFORMED.
Summary
This PR addresses potential deadlocks during WebRTC session cleanup.
close().PeerConnectionidentity instead of SDP fingerprint, preventing delayed callbacks from removing replacement sessions.Summary by CodeRabbit