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>
This commit is contained in:
Sebastian Dröge 2024-08-08 16:44:16 +03:00 committed by GStreamer Marge Bot
parent 0da1c8e9c9
commit bc930122ba

View file

@ -890,7 +890,15 @@ impl BaseWebRTCSrc {
gst::info!(CAT, imp = self, "unpreparing"); gst::info!(CAT, imp = self, "unpreparing");
let mut state = self.state.lock().unwrap(); 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() { for (_, s) in sessions.iter() {
let id = s.id.as_str(); let id = s.id.as_str();
let bin = s.webrtcbin().parent().and_downcast::<gst::Bin>().unwrap(); 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 // 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 // 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. // but the pad-removed handler is also taking the state lock.
let sessions = mem::take(&mut state.sessions);
drop(state);
drop(sessions); drop(sessions);
self.maybe_stop_signaller(); self.maybe_stop_signaller();
@ -958,8 +964,8 @@ impl BaseWebRTCSrc {
instance, instance,
move |_signaler: glib::Object, session_id: &str| { move |_signaler: glib::Object, session_id: &str| {
let this = instance.imp(); let this = instance.imp();
let state = this.state.lock().unwrap(); let mut state = this.state.lock().unwrap();
let Some(session) = state.sessions.get(session_id) else { let Some(session) = state.sessions.remove(session_id) else {
gst::error!( gst::error!(
CAT, CAT,
imp = this, imp = this,
@ -993,7 +999,6 @@ impl BaseWebRTCSrc {
// Drop session after releasing the state lock. Dropping the session can release its bin, // 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 // and during release of the bin its pads are removed but the pad-removed handler is also
// taking the state lock. // taking the state lock.
let session = this.state.lock().unwrap().sessions.remove(session_id);
drop(session); drop(session);
true true