timer: fix race when reschedulingr (fixes #17289)
authorRémi Denis-Courmont <remi@remlab.net>
Wed, 17 Aug 2016 11:12:30 +0000 (14:12 +0300)
committerRémi Denis-Courmont <remi@remlab.net>
Wed, 17 Aug 2016 12:19:12 +0000 (15:19 +0300)
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.

src/posix/timer.c

index 698f486..73d84de 100644 (file)
@@ -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 ();