splitmuxsrc: Use an RW lock instead of a mutex to protect the pad list

Fix a deadlock around the pads list by using an RW lock to
allow simultaneous readers. The pad list doesn't really changes
except at startup and shutdown.
This commit is contained in:
Jan Schmidt 2019-06-11 18:40:09 +10:00
parent 26d6532702
commit 86c131b668
2 changed files with 25 additions and 23 deletions

View file

@ -241,7 +241,7 @@ static void
gst_splitmux_src_init (GstSplitMuxSrc * splitmux) gst_splitmux_src_init (GstSplitMuxSrc * splitmux)
{ {
g_mutex_init (&splitmux->lock); g_mutex_init (&splitmux->lock);
g_mutex_init (&splitmux->pads_lock); g_rw_lock_init (&splitmux->pads_rwlock);
splitmux->total_duration = GST_CLOCK_TIME_NONE; splitmux->total_duration = GST_CLOCK_TIME_NONE;
gst_segment_init (&splitmux->play_segment, GST_FORMAT_TIME); gst_segment_init (&splitmux->play_segment, GST_FORMAT_TIME);
} }
@ -252,7 +252,7 @@ gst_splitmux_src_dispose (GObject * object)
GstSplitMuxSrc *splitmux = GST_SPLITMUX_SRC (object); GstSplitMuxSrc *splitmux = GST_SPLITMUX_SRC (object);
GList *cur; GList *cur;
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_WLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
@ -262,7 +262,7 @@ gst_splitmux_src_dispose (GObject * object)
g_list_free (splitmux->pads); g_list_free (splitmux->pads);
splitmux->n_pads = 0; splitmux->n_pads = 0;
splitmux->pads = NULL; splitmux->pads = NULL;
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_WUNLOCK (splitmux);
G_OBJECT_CLASS (parent_class)->dispose (object); G_OBJECT_CLASS (parent_class)->dispose (object);
} }
@ -272,7 +272,7 @@ gst_splitmux_src_finalize (GObject * object)
{ {
GstSplitMuxSrc *splitmux = GST_SPLITMUX_SRC (object); GstSplitMuxSrc *splitmux = GST_SPLITMUX_SRC (object);
g_mutex_clear (&splitmux->lock); g_mutex_clear (&splitmux->lock);
g_mutex_clear (&splitmux->pads_lock); g_rw_lock_clear (&splitmux->pads_rwlock);
g_free (splitmux->location); g_free (splitmux->location);
G_OBJECT_CLASS (parent_class)->finalize (object); G_OBJECT_CLASS (parent_class)->finalize (object);
@ -771,11 +771,11 @@ gst_splitmux_pad_loop (GstPad * pad)
guint n_notlinked; guint n_notlinked;
/* Only post not-linked if all pads are not-linked */ /* Only post not-linked if all pads are not-linked */
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
n_notlinked = count_not_linked (splitmux); n_notlinked = count_not_linked (splitmux);
post_error = (splitmux->pads_complete post_error = (splitmux->pads_complete
&& n_notlinked == splitmux->n_pads); && n_notlinked == splitmux->n_pads);
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
if (post_error) if (post_error)
GST_ELEMENT_FLOW_ERROR (splitmux, ret); GST_ELEMENT_FLOW_ERROR (splitmux, ret);
@ -812,7 +812,7 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part,
&splitmux->play_segment, extra_flags)) &splitmux->play_segment, extra_flags))
return FALSE; return FALSE;
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
SplitMuxSrcPad *splitpad = (SplitMuxSrcPad *) (cur->data); SplitMuxSrcPad *splitpad = (SplitMuxSrcPad *) (cur->data);
@ -831,7 +831,7 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part,
gst_pad_start_task (GST_PAD (splitpad), gst_pad_start_task (GST_PAD (splitpad),
(GstTaskFunction) gst_splitmux_pad_loop, splitpad, NULL); (GstTaskFunction) gst_splitmux_pad_loop, splitpad, NULL);
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
return TRUE; return TRUE;
} }
@ -976,10 +976,10 @@ gst_splitmux_src_stop (GstSplitMuxSrc * splitmux)
splitmux->parts[i] = NULL; splitmux->parts[i] = NULL;
} }
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_WLOCK (splitmux);
pads_list = splitmux->pads; pads_list = splitmux->pads;
splitmux->pads = NULL; splitmux->pads = NULL;
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_WUNLOCK (splitmux);
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
for (cur = g_list_first (pads_list); cur != NULL; cur = g_list_next (cur)) { for (cur = g_list_first (pads_list); cur != NULL; cur = g_list_next (cur)) {
@ -1038,7 +1038,7 @@ gst_splitmux_find_output_pad (GstSplitMuxPartReader * part, GstPad * pad,
gboolean is_new_pad = FALSE; gboolean is_new_pad = FALSE;
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_WLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
GstPad *tmp = (GstPad *) (cur->data); GstPad *tmp = (GstPad *) (cur->data);
@ -1065,7 +1065,7 @@ gst_splitmux_find_output_pad (GstSplitMuxPartReader * part, GstPad * pad,
&splitmux_and_pad); &splitmux_and_pad);
is_new_pad = TRUE; is_new_pad = TRUE;
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_WUNLOCK (splitmux);
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
g_free (pad_name); g_free (pad_name);
@ -1096,14 +1096,14 @@ gst_splitmux_push_event (GstSplitMuxSrc * splitmux, GstEvent * e,
gst_event_set_seqnum (e, seqnum); gst_event_set_seqnum (e, seqnum);
} }
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
GstPad *pad = GST_PAD_CAST (cur->data); GstPad *pad = GST_PAD_CAST (cur->data);
gst_event_ref (e); gst_event_ref (e);
gst_pad_push_event (pad, e); gst_pad_push_event (pad, e);
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
gst_event_unref (e); gst_event_unref (e);
} }
@ -1119,7 +1119,7 @@ gst_splitmux_push_flush_stop (GstSplitMuxSrc * splitmux, guint32 seqnum)
gst_event_set_seqnum (e, seqnum); gst_event_set_seqnum (e, seqnum);
} }
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
SplitMuxSrcPad *target = (SplitMuxSrcPad *) (cur->data); SplitMuxSrcPad *target = (SplitMuxSrcPad *) (cur->data);
@ -1130,7 +1130,7 @@ gst_splitmux_push_flush_stop (GstSplitMuxSrc * splitmux, guint32 seqnum)
target->sent_stream_start = FALSE; target->sent_stream_start = FALSE;
target->sent_segment = FALSE; target->sent_segment = FALSE;
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
gst_event_unref (e); gst_event_unref (e);
} }
@ -1346,7 +1346,7 @@ splitmux_src_pad_event (GstPad * pad, GstObject * parent, GstEvent * event)
gst_splitmux_push_event (splitmux, gst_event_new_flush_start (), seqnum); gst_splitmux_push_event (splitmux, gst_event_new_flush_start (), seqnum);
/* Stop all parts, which will work because of the flush */ /* Stop all parts, which will work because of the flush */
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
for (cur = g_list_first (splitmux->pads); for (cur = g_list_first (splitmux->pads);
cur != NULL; cur = g_list_next (cur)) { cur != NULL; cur = g_list_next (cur)) {
@ -1362,7 +1362,7 @@ splitmux_src_pad_event (GstPad * pad, GstObject * parent, GstEvent * event)
GstPad *splitpad = (GstPad *) (cur->data); GstPad *splitpad = (GstPad *) (cur->data);
gst_pad_pause_task (GST_PAD (splitpad)); gst_pad_pause_task (GST_PAD (splitpad));
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
/* Send flush stop */ /* Send flush stop */
@ -1424,11 +1424,11 @@ splitmux_src_pad_query (GstPad * pad, GstObject * parent, GstQuery * query)
SplitMuxSrcPad *anypad; SplitMuxSrcPad *anypad;
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_RLOCK (splitmux);
anypad = (SplitMuxSrcPad *) (splitmux->pads->data); anypad = (SplitMuxSrcPad *) (splitmux->pads->data);
part = splitmux->parts[anypad->cur_part]; part = splitmux->parts[anypad->cur_part];
ret = gst_splitmux_part_reader_src_query (part, pad, query); ret = gst_splitmux_part_reader_src_query (part, pad, query);
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_RUNLOCK (splitmux);
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
break; break;
} }

View file

@ -57,7 +57,7 @@ struct _GstSplitMuxSrc
gboolean async_pending; gboolean async_pending;
gboolean pads_complete; gboolean pads_complete;
GMutex pads_lock; GRWLock pads_rwlock;
GList *pads; /* pads_lock */ GList *pads; /* pads_lock */
guint n_pads; guint n_pads;
guint n_notlinked; guint n_notlinked;
@ -108,8 +108,10 @@ gboolean register_splitmuxsrc (GstPlugin * plugin);
#define SPLITMUX_SRC_LOCK(s) g_mutex_lock(&(s)->lock) #define SPLITMUX_SRC_LOCK(s) g_mutex_lock(&(s)->lock)
#define SPLITMUX_SRC_UNLOCK(s) g_mutex_unlock(&(s)->lock) #define SPLITMUX_SRC_UNLOCK(s) g_mutex_unlock(&(s)->lock)
#define SPLITMUX_SRC_PADS_LOCK(s) g_mutex_lock(&(s)->pads_lock) #define SPLITMUX_SRC_PADS_WLOCK(s) g_rw_lock_writer_lock(&(s)->pads_rwlock)
#define SPLITMUX_SRC_PADS_UNLOCK(s) g_mutex_unlock(&(s)->pads_lock) #define SPLITMUX_SRC_PADS_WUNLOCK(s) g_rw_lock_writer_unlock(&(s)->pads_rwlock)
#define SPLITMUX_SRC_PADS_RLOCK(s) g_rw_lock_reader_lock(&(s)->pads_rwlock)
#define SPLITMUX_SRC_PADS_RUNLOCK(s) g_rw_lock_reader_unlock(&(s)->pads_rwlock)
G_END_DECLS G_END_DECLS