diff options
author | François Laignel <francois@centricular.com> | 2023-05-01 14:14:25 +0200 |
---|---|---|
committer | GStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org> | 2023-05-02 21:56:39 +0000 |
commit | 5ef2ce69ff1fe6d33b60689e69a3be43f16faf82 (patch) | |
tree | 2cc54bac4674d2b537feab522389c110deec3574 /subprojects | |
parent | 5a7ed3c89d2cdcd62a6b726629de9e3bf29fdfba (diff) | |
download | gstreamer-5ef2ce69ff1fe6d33b60689e69a3be43f16faf82.tar.gz |
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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4528>
Diffstat (limited to 'subprojects')
-rw-r--r-- | subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c | 2 |
1 files changed, 2 insertions, 0 deletions
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); } /** |