From 5ef2ce69ff1fe6d33b60689e69a3be43f16faf82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Mon, 1 May 2023 14:14:25 +0200 Subject: [PATCH] rtpmanager/rtsession: race conditions leading to critical warnings While testing the [implementation for insertable streams] in `webrtcsink` & `webrtcsrc`, I encountered critical warnings, which turned out to result from two race conditions in `rtpsession`. Both race conditions produce: > GLib-CRITICAL: g_hash_table_foreach: > assertion 'version == hash_table->version' failed This commit fixes one of the race conditions observed. In its simplest form, the test consists in 2 pipelines and a Signalling server: * pipelines_sink: audiotestsrc ! webrtcsink * pipelines_src: webrtcsrc ! appsrc 1. Set `pipelines_sink` to `Playing`. 2. The Signalling server delivers the `producer_id`. 3. Initialize `pipelines_src` to establish a session with `producer_id`. 4. Set `pipelines_src` to `Playing`. 5. Wait for a buffer to be received by the `appsrc`. 6. Set `pipelines_src` to `Null`. 7. Set `pipelines_sink` to `Null`. The race condition happens in the following sequence: * `webrtcsink` runs a task to periodically retrieve statistics from `webrtcbin`. This transitively ends up executing `rtp_session_create_stats`. * `pipelines_sink` is set to `Null`. * In `Paused` to `Ready`, `gst_rtp_session_change_state()` calls `rtp_session_reset()`. * The assertion failure occurs when `rtp_session_reset` is called while `rtp_session_create_stats` is executing. This is because `rtp_session_create_stats` acquires the lock on `session` prior to calling `g_hash_table_foreach`, but `rtp_session_reset` doesn't acquire the lock before calling `g_hash_table_remove_all`. Acquiring the lock in `rtp_session_reset` fixes the issue. [implementing insertable streams support]: https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1176 Part-of: --- subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c b/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c index 269059a2f3..7dc5eb6106 100644 --- a/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c +++ b/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c @@ -1189,6 +1189,7 @@ rtp_session_reset (RTPSession * sess) { g_return_if_fail (RTP_IS_SESSION (sess)); + RTP_SESSION_LOCK (sess); /* remove all sources */ g_hash_table_remove_all (sess->ssrcs[sess->mask_idx]); sess->total_sources = 0; @@ -1217,6 +1218,7 @@ rtp_session_reset (RTPSession * sess) g_list_free_full (sess->conflicting_addresses, (GDestroyNotify) rtp_conflicting_address_free); sess->conflicting_addresses = NULL; + RTP_SESSION_UNLOCK (sess); } /**