systemclock: Don't keep the clock entry locked while getting the time from the clock
gst_clock_get_time() will take the clock mutex, which would then result in a lock order violation and possible deadlocks. If both mutexes are to be locked, the clock must always be locked first. Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7994>
This commit is contained in:
parent
ec698179a9
commit
1ee349f986
@ -1126,16 +1126,26 @@ gst_system_clock_id_wait_jitter_unlocked (GstClock * clock,
|
|||||||
GstClockReturn status;
|
GstClockReturn status;
|
||||||
gint64 mono_ts;
|
gint64 mono_ts;
|
||||||
|
|
||||||
status = GST_CLOCK_ENTRY_STATUS (entry);
|
/* Getting the time from the clock locks the clock, so without unlocking the
|
||||||
if (G_UNLIKELY (status == GST_CLOCK_UNSCHEDULED)) {
|
* entry we would have a lock order violation here that can lead to deadlocks.
|
||||||
return GST_CLOCK_UNSCHEDULED;
|
*
|
||||||
}
|
* It's not a problem to take the mutex again after getting the times (which
|
||||||
|
* might block for a moment) as waiting happens based on the absolute time.
|
||||||
|
*/
|
||||||
|
GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) entry);
|
||||||
|
|
||||||
/* need to call the overridden method because we want to sync against the time
|
/* need to call the overridden method because we want to sync against the time
|
||||||
* of the clock, whatever the subclass uses as a clock. */
|
* of the clock, whatever the subclass uses as a clock. */
|
||||||
now = gst_clock_get_time (clock);
|
now = gst_clock_get_time (clock);
|
||||||
mono_ts = g_get_monotonic_time ();
|
mono_ts = g_get_monotonic_time ();
|
||||||
|
|
||||||
|
GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) entry);
|
||||||
|
/* Might have been unscheduled in the meantime */
|
||||||
|
status = GST_CLOCK_ENTRY_STATUS (entry);
|
||||||
|
if (G_UNLIKELY (status == GST_CLOCK_UNSCHEDULED)) {
|
||||||
|
return GST_CLOCK_UNSCHEDULED;
|
||||||
|
}
|
||||||
|
|
||||||
/* get the time of the entry */
|
/* get the time of the entry */
|
||||||
entryt = GST_CLOCK_ENTRY_TIME (entry);
|
entryt = GST_CLOCK_ENTRY_TIME (entry);
|
||||||
|
|
||||||
@ -1223,10 +1233,20 @@ gst_system_clock_id_wait_jitter_unlocked (GstClock * clock,
|
|||||||
"entry %p unlocked after timeout", entry);
|
"entry %p unlocked after timeout", entry);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) entry);
|
||||||
|
|
||||||
/* reschedule if gst_cond_wait_until returned early or we have to reschedule after
|
/* reschedule if gst_cond_wait_until returned early or we have to reschedule after
|
||||||
* an unlock*/
|
* an unlock*/
|
||||||
mono_ts = g_get_monotonic_time ();
|
|
||||||
now = gst_clock_get_time (clock);
|
now = gst_clock_get_time (clock);
|
||||||
|
mono_ts = g_get_monotonic_time ();
|
||||||
|
|
||||||
|
GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) entry);
|
||||||
|
/* Might have been unscheduled in the meantime */
|
||||||
|
status = GST_CLOCK_ENTRY_STATUS (entry);
|
||||||
|
if (G_UNLIKELY (status == GST_CLOCK_UNSCHEDULED)) {
|
||||||
|
goto done;
|
||||||
|
}
|
||||||
|
|
||||||
diff = GST_CLOCK_DIFF (now, entryt);
|
diff = GST_CLOCK_DIFF (now, entryt);
|
||||||
|
|
||||||
if (diff <= CLOCK_MIN_WAIT_TIME) {
|
if (diff <= CLOCK_MIN_WAIT_TIME) {
|
||||||
|
Loading…
x
Reference in New Issue
Block a user