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>
This commit is contained in:
François Laignel 2023-05-01 14:14:25 +02:00 committed by GStreamer Marge Bot
parent 5a7ed3c89d
commit 5ef2ce69ff

View file

@ -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);
}
/**