From bc930122ba15b8d7bee797154513fc5b4bd64fb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Thu, 8 Aug 2024 16:44:16 +0300
Subject: [PATCH] webrtcsrc: Make sure to always call `end_session()` without
 the state lock

This was already done in another place for the same reason: preventing a
deadlock. It's probably not correct as hinted by the FIXME comment but
better than deadlocking at least.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1701>
---
 net/webrtc/src/webrtcsrc/imp.rs | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/webrtc/src/webrtcsrc/imp.rs b/net/webrtc/src/webrtcsrc/imp.rs
index 72c06c72..a0a8d182 100644
--- a/net/webrtc/src/webrtcsrc/imp.rs
+++ b/net/webrtc/src/webrtcsrc/imp.rs
@@ -890,7 +890,15 @@ impl BaseWebRTCSrc {
         gst::info!(CAT, imp = self, "unpreparing");
 
         let mut state = self.state.lock().unwrap();
-        let sessions = &state.sessions;
+        let sessions = mem::take(&mut state.sessions);
+        drop(state);
+
+        // FIXME: Needs a safer way to perform end_session.
+        // We had to release the lock during the session tear down
+        // to avoid blocking the session bin pad's chain function which could potentially
+        // block the end_session while removing the bin from webrtcsrc, causing a deadlock
+        // This looks unsafe to end a session without holding the state lock
+
         for (_, s) in sessions.iter() {
             let id = s.id.as_str();
             let bin = s.webrtcbin().parent().and_downcast::<gst::Bin>().unwrap();
@@ -902,8 +910,6 @@ impl BaseWebRTCSrc {
         // Drop all sessions after releasing the state lock. Dropping the sessions
         // can release their bin, and during release of the bin its pads are removed
         // but the pad-removed handler is also taking the state lock.
-        let sessions = mem::take(&mut state.sessions);
-        drop(state);
         drop(sessions);
 
         self.maybe_stop_signaller();
@@ -958,8 +964,8 @@ impl BaseWebRTCSrc {
                         instance,
                         move |_signaler: glib::Object, session_id: &str| {
                             let this = instance.imp();
-                            let state = this.state.lock().unwrap();
-                            let Some(session) = state.sessions.get(session_id) else {
+                            let mut state = this.state.lock().unwrap();
+                            let Some(session) = state.sessions.remove(session_id) else {
                                 gst::error!(
                                     CAT,
                                     imp = this,
@@ -993,7 +999,6 @@ impl BaseWebRTCSrc {
                             // Drop session after releasing the state lock. Dropping the session can release its bin,
                             // and during release of the bin its pads are removed but the pad-removed handler is also
                             // taking the state lock.
-                            let session = this.state.lock().unwrap().sessions.remove(session_id);
                             drop(session);
 
                             true