Fix raise(3) so it works in signal handlers.
We could special-case raise(3) in non-threaded programs, but the more
conservative course is to make pthread_kill(3) work in signal handlers
at the cost of a race shared by other C libraries.
Change-Id: I59fb23d03bdabf403435e731704b33acdf3e0234
diff --git a/libc/bionic/pthread_accessor.h b/libc/bionic/pthread_accessor.h
index eb8c350..2a320f6 100644
--- a/libc/bionic/pthread_accessor.h
+++ b/libc/bionic/pthread_accessor.h
@@ -36,6 +36,14 @@
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 @@
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&);
diff --git a/libc/bionic/pthread_kill.cpp b/libc/bionic/pthread_kill.cpp
index 2d37ae9..54f71ee 100644
--- a/libc/bionic/pthread_kill.cpp
+++ b/libc/bionic/pthread_kill.cpp
@@ -42,7 +42,11 @@
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;
}
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 239092c..98bc4e5 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -252,6 +252,24 @@
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);
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index b100372..e41dc1c 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -76,6 +76,25 @@
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 @@
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;
- sigemptyset(&action.sa_mask);
- action.sa_flags = 0;
- action.sa_handler = HandleSIGALRM;
- sigaction(SIGALRM, &action, NULL);
+ ScopedSignalHandler ssh(SIGALRM, HandleSIGALRM);
sigset_t wait_set;
sigemptyset(&wait_set);
