commit 62d84b19359a8ddd3df5b6293d1b05ef5281f532
author Christopher Ferris <cferris@google.com> Mon Oct 20 19:09:19 2014 -0700
committer Christopher Ferris <cferris@google.com> Wed Oct 22 13:20:39 2014 -0700
tree b614f0c9c9671eb6beaec448a7d22dc1ad44e298
parent 098cf45f4e853f3c85c14af0e475bfae0839f027

Fix race condition in timer disarm/delete.

When setting a repeat timer using the SIGEV_THREAD mechanism, it's possible
that the callback can be called after the timer is disarmed or deleted.
This happens because the kernel can generate signals that the timer thread
will continue to handle even after the timer is supposed to be off.

Add two new tests to verify that disarming/deleting doesn't continue to
call the callback.

Modify the repeat test to finish more quickly than before.

Refactor the Counter implementation a bit.

Bug: 18039727

(cherry pick from commit 0724132c3263145f2a667f453a199d313a5b3d9f)

Change-Id: I135726ea4038a47920a6c511708813b1a9996c42

libc/bionic/posix_timers.cpp

diff --git a/libc/bionic/posix_timers.cpp b/libc/bionic/posix_timers.cpp
index 7ad0ef1..3c664d9 100644
--- a/libc/bionic/posix_timers.cpp
+++ b/libc/bionic/posix_timers.cpp
@@ -62,6 +62,7 @@
   pthread_t callback_thread;
   void (*callback)(sigval_t);
   sigval_t callback_argument;
+  volatile bool armed;
 };
 
 static __kernel_timer_t to_kernel_timer_id(timer_t timer) {
@@ -83,7 +84,7 @@
       continue;
     }
 
-    if (si.si_code == SI_TIMER) {
+    if (si.si_code == SI_TIMER && timer->armed) {
       // This signal was sent because a timer fired, so call the callback.
       timer->callback(timer->callback_argument);
     } else if (si.si_code == SI_TKILL) {
@@ -95,6 +96,9 @@
 }
 
 static void __timer_thread_stop(PosixTimer* timer) {
+  // Immediately mark the timer as disarmed so even if some events
+  // continue to happen, the callback won't be called.
+  timer->armed = false;
   pthread_kill(timer->callback_thread, TIMER_SIGNAL);
 }
 
@@ -121,6 +125,7 @@
   // Otherwise, this must be SIGEV_THREAD timer...
   timer->callback = evp->sigev_notify_function;
   timer->callback_argument = evp->sigev_value;
+  timer->armed = false;
 
   // Check arguments that the kernel doesn't care about but we do.
   if (timer->callback == NULL) {
@@ -200,7 +205,18 @@
 
 // http://pubs.opengroup.org/onlinepubs/9699919799/functions/timer_getoverrun.html
 int timer_settime(timer_t id, int flags, const itimerspec* ts, itimerspec* ots) {
-  return __timer_settime(to_kernel_timer_id(id), flags, ts, ots);
+  PosixTimer* timer= reinterpret_cast<PosixTimer*>(id);
+  int rc = __timer_settime(timer->kernel_timer_id, flags, ts, ots);
+  if (rc == 0) {
+    // Mark the timer as either being armed or disarmed. This avoids the
+    // callback being called after the disarm for SIGEV_THREAD timers only.
+    if (ts->it_value.tv_sec != 0 || ts->it_value.tv_nsec != 0) {
+      timer->armed = true;
+    } else {
+      timer->armed = false;
+    }
+  }
+  return rc;
 }
 
 // http://pubs.opengroup.org/onlinepubs/9699919799/functions/timer_getoverrun.html