am 719d46f8: am a9ff09d1: Merge "Fix raise(3) so it works in signal handlers."

* commit '719d46f8ac4f2af89f8e8927db8c460b91622417':
  Fix raise(3) so it works in signal handlers.

libc/bionic/pthread_accessor.h

@@ -36,6 +36,14 @@ class pthread_accessor {
     Unlock();
   }
 
+  void Unlock() {
+    if (is_locked_) {
+      is_locked_ = false;
+      thread_ = NULL;
+      pthread_mutex_unlock(&gThreadListLock);
+    }
+  }
+
   pthread_internal_t& operator*() const { return *thread_; }
   pthread_internal_t* operator->() const { return thread_; }
   pthread_internal_t* get() const { return thread_; }
@@ -49,14 +57,6 @@ class pthread_accessor {
     is_locked_ = true;
   }
 
-  void Unlock() {
-    if (is_locked_) {
-      is_locked_ = false;
-      thread_ = NULL;
-      pthread_mutex_unlock(&gThreadListLock);
-    }
-  }
-
   // Disallow copy and assignment.
   pthread_accessor(const pthread_accessor&);
   void operator=(const pthread_accessor&);

libc/bionic/pthread_kill.cpp

@@ -42,7 +42,11 @@ int pthread_kill(pthread_t t, int sig) {
     return ESRCH;
   }
 
-  int rc = tgkill(getpid(), thread->tid, sig);
+  // There's a race here, but it's one we share with all other C libraries.
+  pid_t tid = thread->tid;
+  thread.Unlock();
+
+  int rc = tgkill(getpid(), tid, sig);
   if (rc == -1) {
     return errno;
   }

tests/pthread_test.cpp

@@ -252,6 +252,24 @@ TEST(pthread, pthread_kill__invalid_signal) {
   ASSERT_EQ(EINVAL, pthread_kill(pthread_self(), -1));
 }
 
+static void pthread_kill__in_signal_handler_helper(int signal_number) {
+  static int count = 0;
+  ASSERT_EQ(SIGALRM, signal_number);
+  if (++count == 1) {
+    // Can we call pthread_kill from a signal handler?
+    ASSERT_EQ(0, pthread_kill(pthread_self(), SIGALRM));
+  }
+}
+
+TEST(pthread, pthread_kill__in_signal_handler) {
+  struct sigaction action;
+  sigemptyset(&action.sa_mask);
+  action.sa_flags = 0;
+  action.sa_handler = pthread_kill__in_signal_handler_helper;
+  sigaction(SIGALRM, &action, NULL);
+  ASSERT_EQ(0, pthread_kill(pthread_self(), SIGALRM));
+}
+
 TEST(pthread, pthread_detach__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);

tests/signal_test.cpp

@@ -76,6 +76,25 @@ static void TestSigSet2(Fn fn) {
   ASSERT_EQ(0, errno);
 }
 
+class ScopedSignalHandler {
+ public:
+  ScopedSignalHandler(int signal_number, void (*handler)(int)) : signal_number_(signal_number) {
+    sigemptyset(&action_.sa_mask);
+    action_.sa_flags = 0;
+    action_.sa_handler = handler;
+    sigaction(signal_number_, &action_, &old_action_);
+  }
+
+  ~ScopedSignalHandler() {
+    sigaction(signal_number_, &old_action_, NULL);
+  }
+
+ private:
+  struct sigaction action_;
+  struct sigaction old_action_;
+  const int signal_number_;
+};
+
 TEST(signal, sigismember_invalid) {
   TestSigSet2(sigismember);
 }
@@ -102,16 +121,25 @@ TEST(signal, raise_invalid) {
   ASSERT_EQ(EINVAL, errno);
 }
 
+static void raise_in_signal_handler_helper(int signal_number) {
+  ASSERT_EQ(SIGALRM, signal_number);
+  static int count = 0;
+  if (++count == 1) {
+    raise(SIGALRM);
+  }
+}
+
+TEST(signal, raise_in_signal_handler) {
+  ScopedSignalHandler ssh(SIGALRM, raise_in_signal_handler_helper);
+  raise(SIGALRM);
+}
+
 static void HandleSIGALRM(int signal_number) {
   ASSERT_EQ(SIGALRM, signal_number);
 }
 
 TEST(signal, sigwait) {
-  struct sigaction action;
+  ScopedSignalHandler ssh(SIGALRM, HandleSIGALRM);
-  sigemptyset(&action.sa_mask);
-  action.sa_flags = 0;
-  action.sa_handler = HandleSIGALRM;
-  sigaction(SIGALRM, &action, NULL);
 
   sigset_t wait_set;
   sigemptyset(&wait_set);