[vlc-commits] timer: fix race when reschedulingr (fixes #17289)

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Aug 17 14:12:30 2016 +0300| [e7c0cb0f653afce070625c0685a2f04e05db91ec] | committer: Rémi Denis-Courmont

timer: fix race when reschedulingr (fixes #17289)

This fixes the overrun counter to always use the intended deadline
and interval values. Otherwise the counter value would be garbade, and
potentially cause an interger underflow. This would also potentially
corrupt the adjusted deadline and stall the timer thread.

This also fixes the disarm handling. From now on, a non-recurrent timer
is disarmed after expiration only if it has not been rescheduled or if
the new schedule is no later than the old one.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=e7c0cb0f653afce070625c0685a2f04e05db91ec
---

 src/posix/timer.c | 49 +++++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/src/posix/timer.c b/src/posix/timer.c
index 698f486..73d84de 100644
--- a/src/posix/timer.c
+++ b/src/posix/timer.c
@@ -65,36 +65,41 @@ static void *vlc_timer_thread (void *data)
             vlc_cond_wait (&timer->reschedule, &timer->lock);
         }
 
-        if (vlc_cond_timedwait (&timer->reschedule, &timer->lock,
-                                timer->value) == 0)
+        if (timer->interval != 0)
+        {
+            mtime_t now = mdate();
+
+            if (now > timer->value)
+            {   /* Update overrun counter */
+                unsigned misses = (now - timer->value) / timer->interval;
+
+                timer->value += misses * timer->interval;
+                assert(timer->value <= now);
+                atomic_fetch_add_explicit(&timer->overruns, misses,
+                                          memory_order_relaxed);
+            }
+        }
+
+        mtime_t value = timer->value;
+
+        if (vlc_cond_timedwait(&timer->reschedule, &timer->lock, value) == 0)
             continue;
-        if (timer->interval == 0)
-            timer->value = 0; /* disarm */
+
+        if (likely(timer->value <= value))
+        {
+            timer->value += timer->interval; /* rearm */
+
+            if (timer->interval == 0)
+                timer->value = 0; /* disarm */
+        }
+
         vlc_mutex_unlock (&timer->lock);
 
         int canc = vlc_savecancel ();
         timer->func (timer->data);
         vlc_restorecancel (canc);
 
-        mtime_t now = mdate ();
-        unsigned misses;
-
         vlc_mutex_lock (&timer->lock);
-        if (timer->interval == 0)
-            continue;
-
-        misses = (now - timer->value) / timer->interval;
-        timer->value += timer->interval;
-        /* Try to compensate for one miss (mwait() will return immediately)
-         * but no more. Otherwise, we might busy loop, after extended periods
-         * without scheduling (suspend, SIGSTOP, RT preemption, ...). */
-        if (misses > 1)
-        {
-            misses--;
-            timer->value += misses * timer->interval;
-            atomic_fetch_add_explicit (&timer->overruns, misses,
-                                       memory_order_relaxed);
-        }
     }
 
     vlc_cleanup_pop ();