From 3e924f5df9a5975bc1e24da398c9dbd3360d6b22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 22 May 2020 18:12:55 +0300
Subject: [PATCH] systemclock: Don't start waiting for a clock id if it was
 signalled before

Otherwise it can happen that unscheduling a clock id never takes place
and instead it is waiting until the normal timeout. This can happen if
the wait thread checks the status and sets it to busy, then the
unschedule thread sets it to unscheduled and signals the condition
variable, and then the waiting thread starts waiting. As condition
variables don't have a state (unlike Windows event objects), we have to
remember ourselves in a new boolean flag protected by the entry mutex
whether it is currently signalled, and reset this after waiting.

Previously this was not a problem because a file descriptor was written
to for waking up, and the token was left on the file descriptor until
the read from it for waiting.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/500>
---
 gst/gstsystemclock.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/gst/gstsystemclock.c b/gst/gstsystemclock.c
index 46a60d0c11..0827e20df6 100644
--- a/gst/gstsystemclock.c
+++ b/gst/gstsystemclock.c
@@ -104,6 +104,7 @@ struct _GstClockEntryFutex
   GDestroyNotify destroy_entry;
 
   gboolean initialized;
+  gboolean signalled;
 
   GMutex lock;
   guint cond_val;
@@ -177,6 +178,7 @@ struct _GstClockEntryPThread
   GDestroyNotify destroy_entry;
 
   gboolean initialized;
+  gboolean signalled;
 
   pthread_cond_t cond;
   pthread_mutex_t lock;
@@ -309,6 +311,7 @@ struct _GstClockEntryGLib
   GDestroyNotify destroy_entry;
 
   gboolean initialized;
+  gboolean signalled;
 
   GMutex lock;
   GCond cond;
@@ -341,6 +344,7 @@ ensure_entry_initialized (GstClockEntryImpl * entry_impl)
   if (!entry_impl->initialized) {
     init_entry (entry_impl);
     entry_impl->initialized = TRUE;
+    entry_impl->signalled = FALSE;
   }
 }
 
@@ -497,7 +501,10 @@ gst_system_clock_dispose (GObject * object)
   if (priv->entries) {
     GstClockEntryImpl *entry = (GstClockEntryImpl *) priv->entries->data;
     ensure_entry_initialized (entry);
+    GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) entry);
+    ((GstClockEntryImpl *) entry)->signalled = TRUE;
     GST_SYSTEM_CLOCK_ENTRY_BROADCAST (entry);
+    GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) entry);
   }
   GST_SYSTEM_CLOCK_UNLOCK (clock);
 
@@ -934,15 +941,20 @@ gst_system_clock_id_wait_jitter_unlocked (GstClock * clock,
 #endif
 
     while (TRUE) {
-      gboolean waitret;
+      gboolean waitret = TRUE;
 
       /* now wait on the entry, it either times out or the cond is signalled.
        * The status of the entry is BUSY only around the wait. */
 
       GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) entry);
-      waitret =
-          GST_SYSTEM_CLOCK_ENTRY_WAIT_UNTIL ((GstClockEntryImpl *) entry,
-          mono_ts * 1000 + diff);
+      while (!((GstClockEntryImpl *) entry)->signalled) {
+        waitret =
+            GST_SYSTEM_CLOCK_ENTRY_WAIT_UNTIL ((GstClockEntryImpl *) entry,
+            mono_ts * 1000 + diff);
+        if (!waitret)
+          break;
+      }
+      ((GstClockEntryImpl *) entry)->signalled = FALSE;
       GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) entry);
 
       /* get the new status, mark as DONE. We do this so that the unschedule
@@ -1194,7 +1206,10 @@ gst_system_clock_id_wait_async (GstClock * clock, GstClockEntry * entry)
          * looks at the new head entry instead, we only need to do this once */
         GST_CAT_DEBUG_OBJECT (GST_CAT_CLOCK, clock,
             "head entry was busy. Wakeup async thread");
+        GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) head);
+        ((GstClockEntryImpl *) head)->signalled = TRUE;
         GST_SYSTEM_CLOCK_ENTRY_BROADCAST ((GstClockEntryImpl *) head);
+        GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) head);
       }
     }
   }
@@ -1244,7 +1259,10 @@ gst_system_clock_id_unschedule (GstClock * clock, GstClockEntry * entry)
     GST_CAT_DEBUG_OBJECT (GST_CAT_CLOCK, clock, "entry was BUSY, doing wakeup");
     if (!entry->unscheduled) {
       ensure_entry_initialized ((GstClockEntryImpl *) entry);
+      GST_SYSTEM_CLOCK_ENTRY_LOCK ((GstClockEntryImpl *) entry);
+      ((GstClockEntryImpl *) entry)->signalled = TRUE;
       GST_SYSTEM_CLOCK_ENTRY_BROADCAST ((GstClockEntryImpl *) entry);
+      GST_SYSTEM_CLOCK_ENTRY_UNLOCK ((GstClockEntryImpl *) entry);
     }
   }
   GST_SYSTEM_CLOCK_UNLOCK (clock);