From 753ad778bc1c3aecc4cd82b8387a7dc8a9b44d34 Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris@google.com>
Date: Thu, 20 Mar 2014 20:47:45 -0700
Subject: [PATCH] Fix deadlock in timer_delete.

If the callback function for a timer did a timer_delete, the function
would never return. The problem was that the timer_delete function would try
to wait until the timer thread has finished. Waiting for yourself to finish
doesn't work very well.

Bug: 13397340
Change-Id: Ica123a5bafbc8660c8a4a909e5c2dead55ca429d
---
 libc/bionic/posix_timers.cpp | 13 ++++++----
 tests/time_test.cpp          | 46 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/libc/bionic/posix_timers.cpp b/libc/bionic/posix_timers.cpp
index ffe213c33..e9190b2c6 100644
--- a/libc/bionic/posix_timers.cpp
+++ b/libc/bionic/posix_timers.cpp
@@ -100,11 +100,14 @@ static void* __timer_thread_start(void* arg) {
 static void __timer_thread_stop(PosixTimer* timer) {
   pthread_kill(timer->callback_thread, TIMER_SIGNAL);
 
-  // We can't pthread_join because POSIX says "the threads created in response to a timer
-  // expiration are created detached, or in an unspecified way if the thread attribute's
-  // detachstate is PTHREAD_CREATE_JOINABLE".
-  while (timer->exiting == 0) {
-    __futex_wait(&timer->exiting, 0, NULL);
+  // If this is being called from within the callback thread, do nothing else.
+  if (pthread_self() != timer->callback_thread) {
+    // We can't pthread_join because POSIX says "the threads created in response to a timer
+    // expiration are created detached, or in an unspecified way if the thread attribute's
+    // detachstate is PTHREAD_CREATE_JOINABLE".
+    while (timer->exiting == 0) {
+      __futex_wait(&timer->exiting, 0, NULL);
+    }
   }
 }
 
diff --git a/tests/time_test.cpp b/tests/time_test.cpp
index af6ae8581..26b7775cc 100644
--- a/tests/time_test.cpp
+++ b/tests/time_test.cpp
@@ -341,3 +341,49 @@ TEST(time, timer_create_multiple) {
   EXPECT_EQ(1, counter2.value);
   EXPECT_EQ(0, counter3.value);
 }
+
+struct TimerDeleteData {
+  timer_t timer_id;
+  pthread_t thread_id;
+  volatile bool complete;
+};
+
+static void TimerDeleteCallback(sigval_t value) {
+  TimerDeleteData* tdd = reinterpret_cast<TimerDeleteData*>(value.sival_ptr);
+
+  tdd->thread_id = pthread_self();
+  timer_delete(tdd->timer_id);
+  tdd->complete = true;
+}
+
+TEST(time, timer_delete_from_timer_thread) {
+  TimerDeleteData tdd;
+  sigevent_t se;
+
+  memset(&se, 0, sizeof(se));
+  se.sigev_notify = SIGEV_THREAD;
+  se.sigev_notify_function = TimerDeleteCallback;
+  se.sigev_value.sival_ptr = &tdd;
+
+  tdd.complete = false;
+  ASSERT_EQ(0, timer_create(CLOCK_REALTIME, &se, &tdd.timer_id));
+
+  itimerspec ts;
+  ts.it_value.tv_sec = 0;
+  ts.it_value.tv_nsec = 100;
+  ts.it_interval.tv_sec = 0;
+  ts.it_interval.tv_nsec = 0;
+  ASSERT_EQ(0, timer_settime(tdd.timer_id, TIMER_ABSTIME, &ts, NULL));
+
+  time_t cur_time = time(NULL);
+  while (!tdd.complete && (time(NULL) - cur_time) < 5);
+  ASSERT_TRUE(tdd.complete);
+
+#if defined(__BIONIC__)
+  // Since bionic timers are implemented by creating a thread to handle the
+  // callback, verify that the thread actually completes.
+  cur_time = time(NULL);
+  while (pthread_detach(tdd.thread_id) != ESRCH && (time(NULL) - cur_time) < 5);
+  ASSERT_EQ(ESRCH, pthread_detach(tdd.thread_id));
+#endif
+}