rtpbin: Make cleaning up sources in rtp_session_on_timeout MT safe

Using _foreach_remove on the hashtable, while releasing the lock protecting
that table inside the callback is not a good idea. The hashtable might
then change (a source removed or added) while signals like on_timeout
are being sent.

This solution makes a copy of the table, performs the _foreach without
actually removing any sources, but marks them for removal on a second
iteration with the real list, but this time not letting go of the lock.

Fixes #630452
This commit is contained in:
Pascal Buhler 2010-09-24 15:33:40 +02:00 committed by Wim Taymans
parent 87a9d7f679
commit ca6a512b5e
3 changed files with 32 additions and 4 deletions

View file

@ -2360,7 +2360,7 @@ session_report_blocks (const gchar * key, RTPSource * source, ReportData * data)
}
/* perform cleanup of sources that timed out */
static gboolean
static void
session_cleanup (const gchar * key, RTPSource * source, ReportData * data)
{
gboolean remove = FALSE;
@ -2428,7 +2428,8 @@ session_cleanup (const gchar * key, RTPSource * source, ReportData * data)
if (sendertimeout)
on_sender_timeout (sess, source);
}
return remove;
source->closing = remove;
}
static void
@ -2556,6 +2557,18 @@ is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
return result;
}
static void
clone_ssrcs_hashtable (gchar * key, RTPSource * source, GHashTable * hash_table)
{
g_hash_table_insert (hash_table, key, g_object_ref (source));
}
static gboolean
remove_closing_sources (const gchar * key, RTPSource * source, gpointer * data)
{
return source->closing;
}
/**
* rtp_session_on_timeout:
* @sess: an #RTPSession
@ -2581,6 +2594,7 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time,
GstFlowReturn result = GST_FLOW_OK;
ReportData data;
RTPSource *own;
GHashTable *table_copy;
gboolean notify = FALSE;
g_return_val_if_fail (RTP_IS_SESSION (sess), GST_FLOW_ERROR);
@ -2602,9 +2616,21 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time,
/* get a new interval, we need this for various cleanups etc */
data.interval = calculate_rtcp_interval (sess, TRUE, sess->first_rtcp);
/* first perform cleanups */
/* Make a local copy of the hashtable. We need to do this because the
* cleanup stage below releases the session lock. */
table_copy = g_hash_table_new_full (NULL, NULL, NULL,
(GDestroyNotify) g_object_unref);
g_hash_table_foreach (sess->ssrcs[sess->mask_idx],
(GHFunc) clone_ssrcs_hashtable, table_copy);
/* Clean up the session, mark the source for removing, this might release the
* session lock. */
g_hash_table_foreach (table_copy, (GHFunc) session_cleanup, &data);
g_hash_table_destroy (table_copy);
/* Now remove the marked sources */
g_hash_table_foreach_remove (sess->ssrcs[sess->mask_idx],
(GHRFunc) session_cleanup, &data);
(GHRFunc) remove_closing_sources, NULL);
/* see if we need to generate SR or RR packets */
if (is_rtcp_time (sess, current_time, &data)) {

View file

@ -158,6 +158,7 @@ rtp_source_init (RTPSource * src)
src->validated = FALSE;
src->internal = FALSE;
src->probation = RTP_DEFAULT_PROBATION;
src->closing = FALSE;
src->sdes = gst_structure_new ("application/x-rtp-source-sdes", NULL);

View file

@ -131,6 +131,7 @@ struct _RTPSource {
gboolean internal;
gboolean is_csrc;
gboolean is_sender;
gboolean closing;
GstStructure *sdes;