Revert^2: "Make sigchain actually own the signal chain.""

This reverts commit 33dca562d5ed9f52f63ced2334c066333ca073e9.

Bug: http://b/36205469
Bug: http://b/36262089
Original-Change-Id: I416b13de800d3e23424cf20688ccfd7e4e48a111
Change-Id: I1267be72c32ed73d1079a572d0b8907556526dde
diff --git a/runtime/fault_handler.cc b/runtime/fault_handler.cc
index 64128cc..4220250 100644
--- a/runtime/fault_handler.cc
+++ b/runtime/fault_handler.cc
@@ -38,8 +38,8 @@
 }
 
 // Signal handler called on SIGSEGV.
-static void art_fault_handler(int sig, siginfo_t* info, void* context) {
-  fault_manager.HandleFault(sig, info, context);
+static bool art_fault_handler(int sig, siginfo_t* info, void* context) {
+  return fault_manager.HandleFault(sig, info, context);
 }
 
 FaultManager::FaultManager() : initialized_(false) {
@@ -49,43 +49,15 @@
 FaultManager::~FaultManager() {
 }
 
-static void SetUpArtAction(struct sigaction* action) {
-  action->sa_sigaction = art_fault_handler;
-  sigemptyset(&action->sa_mask);
-  action->sa_flags = SA_SIGINFO | SA_ONSTACK;
-#if !defined(__APPLE__) && !defined(__mips__)
-  action->sa_restorer = nullptr;
-#endif
-}
-
-void FaultManager::EnsureArtActionInFrontOfSignalChain() {
-  if (initialized_) {
-    struct sigaction action;
-    SetUpArtAction(&action);
-    EnsureFrontOfChain(SIGSEGV, &action);
-  } else {
-    LOG(WARNING) << "Can't call " << __FUNCTION__ << " due to unitialized fault manager";
-  }
-}
-
 void FaultManager::Init() {
   CHECK(!initialized_);
-  struct sigaction action;
-  SetUpArtAction(&action);
-
-  // Set our signal handler now.
-  int e = sigaction(SIGSEGV, &action, &oldaction_);
-  if (e != 0) {
-    VLOG(signals) << "Failed to claim SEGV: " << strerror(errno);
-  }
-  // Make sure our signal handler is called before any user handlers.
-  ClaimSignalChain(SIGSEGV, &oldaction_);
+  AddSpecialSignalHandlerFn(SIGSEGV, art_fault_handler);
   initialized_ = true;
 }
 
 void FaultManager::Release() {
   if (initialized_) {
-    UnclaimSignalChain(SIGSEGV);
+    RemoveSpecialSignalHandlerFn(SIGSEGV, art_fault_handler);
     initialized_ = false;
   }
 }
@@ -118,93 +90,36 @@
   return false;
 }
 
-class ScopedSignalUnblocker {
- public:
-  explicit ScopedSignalUnblocker(const std::initializer_list<int>& signals) {
-    sigset_t new_mask;
-    sigemptyset(&new_mask);
-    for (int signal : signals) {
-      sigaddset(&new_mask, signal);
-    }
-    if (sigprocmask(SIG_UNBLOCK, &new_mask, &previous_mask_) != 0) {
-      PLOG(FATAL) << "failed to unblock signals";
-    }
-  }
-
-  ~ScopedSignalUnblocker() {
-    if (sigprocmask(SIG_SETMASK, &previous_mask_, nullptr) != 0) {
-      PLOG(FATAL) << "failed to unblock signals";
-    }
-  }
-
- private:
-  sigset_t previous_mask_;
-};
-
-class ScopedHandlingSignalSetter {
- public:
-  explicit ScopedHandlingSignalSetter(Thread* thread) : thread_(thread) {
-    CHECK(!thread->HandlingSignal());
-    thread_->SetHandlingSignal(true);
-  }
-
-  ~ScopedHandlingSignalSetter() {
-    CHECK(thread_->HandlingSignal());
-    thread_->SetHandlingSignal(false);
-  }
-
- private:
-  Thread* thread_;
-};
-
-void FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
-  // BE CAREFUL ALLOCATING HERE INCLUDING USING LOG(...)
-  //
-  // If malloc calls abort, it will be holding its lock.
-  // If the handler tries to call malloc, it will deadlock.
-
-  // Use a thread local field to track whether we're recursing, and fall back.
-  // (e.g.. if one of our handlers crashed)
-  Thread* thread = Thread::Current();
-
-  if (thread != nullptr && !thread->HandlingSignal()) {
-    // Unblock some signals and set thread->handling_signal_ to true,
-    // so that we can catch crashes in our signal handler.
-    ScopedHandlingSignalSetter setter(thread);
-    ScopedSignalUnblocker unblocker { SIGABRT, SIGBUS, SIGSEGV }; // NOLINT
-
-    VLOG(signals) << "Handling fault";
+bool FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
+  VLOG(signals) << "Handling fault";
 
 #ifdef TEST_NESTED_SIGNAL
-    // Simulate a crash in a handler.
-    raise(SIGSEGV);
+  // Simulate a crash in a handler.
+  raise(SIGSEGV);
 #endif
 
-    if (IsInGeneratedCode(info, context, true)) {
-      VLOG(signals) << "in generated code, looking for handler";
-      for (const auto& handler : generated_code_handlers_) {
-        VLOG(signals) << "invoking Action on handler " << handler;
-        if (handler->Action(sig, info, context)) {
-          // We have handled a signal so it's time to return from the
-          // signal handler to the appropriate place.
-          return;
-        }
+  if (IsInGeneratedCode(info, context, true)) {
+    VLOG(signals) << "in generated code, looking for handler";
+    for (const auto& handler : generated_code_handlers_) {
+      VLOG(signals) << "invoking Action on handler " << handler;
+      if (handler->Action(sig, info, context)) {
+        // We have handled a signal so it's time to return from the
+        // signal handler to the appropriate place.
+        return true;
       }
+    }
 
-      // We hit a signal we didn't handle.  This might be something for which
-      // we can give more information about so call all registered handlers to
-      // see if it is.
-      if (HandleFaultByOtherHandlers(sig, info, context)) {
-        return;
-      }
+    // We hit a signal we didn't handle.  This might be something for which
+    // we can give more information about so call all registered handlers to
+    // see if it is.
+    if (HandleFaultByOtherHandlers(sig, info, context)) {
+      return true;
     }
   }
 
   // Set a breakpoint in this function to catch unhandled signals.
   art_sigsegv_fault();
-
-  // Pass this on to the next handler in the chain, or the default if none.
-  InvokeUserSignalHandler(sig, info, context);
+  return false;
 }
 
 void FaultManager::AddHandler(FaultHandler* handler, bool generated_code) {
diff --git a/runtime/fault_handler.h b/runtime/fault_handler.h
index ce59ba7..d56cf17 100644
--- a/runtime/fault_handler.h
+++ b/runtime/fault_handler.h
@@ -42,9 +42,9 @@
 
   // Unclaim signals and delete registered handlers.
   void Shutdown();
-  void EnsureArtActionInFrontOfSignalChain();
 
-  void HandleFault(int sig, siginfo_t* info, void* context);
+  // Try to handle a fault, returns true if successful.
+  bool HandleFault(int sig, siginfo_t* info, void* context);
 
   // Added handlers are owned by the fault handler and will be freed on Shutdown().
   void AddHandler(FaultHandler* handler, bool generated_code);
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index a341cdb..b93b8f2 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -39,6 +39,7 @@
 #include "runtime_options.h"
 #include "ScopedLocalRef.h"
 #include "scoped_thread_state_change-inl.h"
+#include "sigchain.h"
 #include "thread-inl.h"
 #include "thread_list.h"
 
@@ -900,7 +901,8 @@
     int version = (*jni_on_load)(this, nullptr);
 
     if (runtime_->GetTargetSdkVersion() != 0 && runtime_->GetTargetSdkVersion() <= 21) {
-      fault_manager.EnsureArtActionInFrontOfSignalChain();
+      // Make sure that sigchain owns SIGSEGV.
+      EnsureFrontOfChain(SIGSEGV);
     }
 
     self->SetClassLoaderOverride(old_class_loader.get());
diff --git a/runtime/native_bridge_art_interface.cc b/runtime/native_bridge_art_interface.cc
index c58854b..d77cfa1 100644
--- a/runtime/native_bridge_art_interface.cc
+++ b/runtime/native_bridge_art_interface.cc
@@ -118,7 +118,7 @@
       for (int signal = 0; signal < _NSIG; ++signal) {
         android::NativeBridgeSignalHandlerFn fn = android::NativeBridgeGetSignalHandler(signal);
         if (fn != nullptr) {
-          SetSpecialSignalHandlerFn(signal, fn);
+          AddSpecialSignalHandlerFn(signal, fn);
         }
       }
 #endif
diff --git a/sigchainlib/sigchain.cc b/sigchainlib/sigchain.cc
index c1efecd..ed6d7cd 100644
--- a/sigchainlib/sigchain.cc
+++ b/sigchainlib/sigchain.cc
@@ -25,6 +25,9 @@
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <ucontext.h>
+
+#include <utility>
 
 #include "sigchain.h"
 
@@ -33,70 +36,24 @@
 #define sighandler_t sig_t
 #endif
 
-namespace art {
+// libsigchain provides an interception layer for signal handlers, to allow ART and others to give
+// their signal handlers the first stab at handling signals before passing them on to user code.
+//
+// It implements wrapper functions for signal, sigaction, and sigprocmask, and a handler that
+// forwards signals appropriately.
+//
+// In our handler, we start off with all signals blocked, fetch the original signal mask from the
+// passed in ucontext, and then adjust our signal mask appropriately for the user handler.
+//
+// It's somewhat tricky for us to properly handle some flag cases:
+//   SA_NOCLDSTOP and SA_NOCLDWAIT: shouldn't matter, we don't have special handlers for SIGCHLD.
+//   SA_NODEFER: unimplemented, we can manually change the signal mask appropriately.
+//  ~SA_ONSTACK: always silently enable this
+//   SA_RESETHAND: unimplemented, but we can probably do this?
+//  ~SA_RESTART: unimplemented, maybe we can reserve an RT signal, register an empty handler that
+//               doesn't have SA_RESTART, and raise the signal to avoid restarting syscalls that are
+//               expected to be interrupted?
 
-typedef int (*SigActionFnPtr)(int, const struct sigaction*, struct sigaction*);
-
-class SignalAction {
- public:
-  SignalAction() : claimed_(false), uses_old_style_(false), special_handler_(nullptr) {
-  }
-
-  // Claim the signal and keep the action specified.
-  void Claim(const struct sigaction& action) {
-    action_ = action;
-    claimed_ = true;
-  }
-
-  // Unclaim the signal and restore the old action.
-  void Unclaim(int signal) {
-    claimed_ = false;
-    sigaction(signal, &action_, nullptr);        // Restore old action.
-  }
-
-  // Get the action associated with this signal.
-  const struct sigaction& GetAction() const {
-    return action_;
-  }
-
-  // Is the signal claimed?
-  bool IsClaimed() const {
-    return claimed_;
-  }
-
-  // Change the recorded action to that specified.
-  // If oldstyle is true then this action is from an older style signal()
-  // call as opposed to sigaction().  In this case the sa_handler is
-  // used when invoking the user's handler.
-  void SetAction(const struct sigaction& action, bool oldstyle) {
-    action_ = action;
-    uses_old_style_ = oldstyle;
-  }
-
-  bool OldStyle() const {
-    return uses_old_style_;
-  }
-
-  void SetSpecialHandler(SpecialSignalHandlerFn fn) {
-    special_handler_ = fn;
-  }
-
-  SpecialSignalHandlerFn GetSpecialHandler() {
-    return special_handler_;
-  }
-
- private:
-  struct sigaction action_;                 // Action to be performed.
-  bool claimed_;                            // Whether signal is claimed or not.
-  bool uses_old_style_;                     // Action is created using signal().  Use sa_handler.
-  SpecialSignalHandlerFn special_handler_;  // A special handler executed before user handlers.
-};
-
-// User's signal handlers
-static SignalAction user_sigactions[_NSIG];
-static bool initialized;
-static void* linked_sigaction_sym;
-static void* linked_sigprocmask_sym;
 
 static void log(const char* format, ...) {
   char buf[256];
@@ -111,102 +68,185 @@
   va_end(ap);
 }
 
-static void CheckSignalValid(int signal) {
-  if (signal <= 0 || signal >= _NSIG) {
-    log("Invalid signal %d", signal);
-    abort();
-  }
-}
+#define fatal(...) log(__VA_ARGS__); abort()
 
-// Sigchainlib's own handler so we can ensure a managed handler is called first even if nobody
-// claimed a chain. Simply forward to InvokeUserSignalHandler.
-static void sigchainlib_managed_handler_sigaction(int sig, siginfo_t* info, void* context) {
-  InvokeUserSignalHandler(sig, info, context);
-}
-
-// Claim a signal chain for a particular signal.
-extern "C" void ClaimSignalChain(int signal, struct sigaction* oldaction) {
-  CheckSignalValid(signal);
-
-  user_sigactions[signal].Claim(*oldaction);
-}
-
-extern "C" void UnclaimSignalChain(int signal) {
-  CheckSignalValid(signal);
-
-  user_sigactions[signal].Unclaim(signal);
-}
-
-// Invoke the user's signal handler.
-extern "C" void InvokeUserSignalHandler(int sig, siginfo_t* info, void* context) {
-  // Check the arguments.
-  CheckSignalValid(sig);
-
-  // The signal must have been claimed in order to get here.  Check it.
-  if (!user_sigactions[sig].IsClaimed()) {
-    abort();
-  }
-
-  // Do we have a managed handler? If so, run it first.
-  SpecialSignalHandlerFn managed = user_sigactions[sig].GetSpecialHandler();
-  if (managed != nullptr) {
-    sigset_t mask, old_mask;
-    sigfillset(&mask);
-    sigprocmask(SIG_BLOCK, &mask, &old_mask);
-    // Call the handler. If it succeeds, we're done.
-    if (managed(sig, info, context)) {
-      sigprocmask(SIG_SETMASK, &old_mask, nullptr);
-      return;
+static int sigorset(sigset_t* dest, sigset_t* left, sigset_t* right) {
+  sigemptyset(dest);
+  for (size_t i = 0; i < sizeof(sigset_t) * CHAR_BIT; ++i) {
+    if (sigismember(left, i) == 1 || sigismember(right, i) == 1) {
+      sigaddset(dest, i);
     }
-    sigprocmask(SIG_SETMASK, &old_mask, nullptr);
+  }
+  return 0;
+}
+
+namespace art {
+
+static decltype(&sigaction) linked_sigaction;
+static decltype(&sigprocmask) linked_sigprocmask;
+__thread bool handling_signal;
+
+class SignalChain {
+ public:
+  SignalChain() : claimed_(false) {
   }
 
-  const struct sigaction& action = user_sigactions[sig].GetAction();
-  if (user_sigactions[sig].OldStyle()) {
-    if (action.sa_handler != nullptr) {
-      action.sa_handler(sig);
-    } else {
-      signal(sig, SIG_DFL);
-      raise(sig);
+  bool IsClaimed() {
+    return claimed_;
+  }
+
+  void Claim(int signo) {
+    if (!claimed_) {
+      Register(signo);
+      claimed_ = true;
     }
+  }
+
+  // Register the signal chain with the kernel if needed.
+  void Register(int signo) {
+    struct sigaction handler_action = {};
+    handler_action.sa_sigaction = SignalChain::Handler;
+    handler_action.sa_flags = SA_RESTART | SA_SIGINFO | SA_ONSTACK;
+    sigfillset(&handler_action.sa_mask);
+    linked_sigaction(signo, &handler_action, &action_);
+  }
+
+  void SetAction(const struct sigaction* action) {
+    action_ = *action;
+  }
+
+  struct sigaction GetAction() {
+    return action_;
+  }
+
+  void AddSpecialHandler(SpecialSignalHandlerFn fn) {
+    for (SpecialSignalHandlerFn& slot : special_handlers_) {
+      if (slot == nullptr) {
+        slot = fn;
+        return;
+      }
+    }
+
+    fatal("too many special signal handlers");
+  }
+
+  void RemoveSpecialHandler(SpecialSignalHandlerFn fn) {
+    // This isn't thread safe, but it's unlikely to be a real problem.
+    size_t len = sizeof(special_handlers_)/sizeof(*special_handlers_);
+    for (size_t i = 0; i < len; ++i) {
+      if (special_handlers_[i] == fn) {
+        for (size_t j = i; j < len - 1; ++j) {
+          special_handlers_[j] = special_handlers_[j + 1];
+        }
+        special_handlers_[len - 1] = nullptr;
+        return;
+      }
+    }
+
+    fatal("failed to find special handler to remove");
+  }
+
+
+  static void Handler(int signo, siginfo_t* siginfo, void*);
+
+ private:
+  bool claimed_;
+  struct sigaction action_;
+  SpecialSignalHandlerFn special_handlers_[2];
+};
+
+static SignalChain chains[_NSIG];
+
+class ScopedFlagSetter {
+ public:
+  explicit ScopedFlagSetter(bool* flag) : flag_(flag) {
+    *flag_ = true;
+  }
+
+  ~ScopedFlagSetter() {
+    *flag_ = false;
+  }
+
+ private:
+  bool* flag_;
+};
+
+class ScopedSignalUnblocker {
+ public:
+  explicit ScopedSignalUnblocker(const std::initializer_list<int>& signals) {
+    sigset_t new_mask;
+    sigemptyset(&new_mask);
+    for (int signal : signals) {
+      sigaddset(&new_mask, signal);
+    }
+    if (sigprocmask(SIG_UNBLOCK, &new_mask, &previous_mask_) != 0) {
+      fatal("failed to unblock signals: %s", strerror(errno));
+    }
+  }
+
+  ~ScopedSignalUnblocker() {
+    if (sigprocmask(SIG_SETMASK, &previous_mask_, nullptr) != 0) {
+      fatal("failed to unblock signals: %s", strerror(errno));
+    }
+  }
+
+ private:
+  sigset_t previous_mask_;
+};
+
+
+void SignalChain::Handler(int signo, siginfo_t* siginfo, void* ucontext_raw) {
+  // Try the special handlers first.
+  // If one of them crashes, we'll reenter this handler and pass that crash onto the user handler.
+  if (!handling_signal) {
+    ScopedFlagSetter flag(&handling_signal);
+    ScopedSignalUnblocker unblocked { SIGABRT, SIGSEGV, SIGBUS }; // NOLINT
+
+    for (const auto& handler : chains[signo].special_handlers_) {
+      if (handler != nullptr && handler(signo, siginfo, ucontext_raw)) {
+        return;
+      }
+    }
+  }
+
+  // Forward to the user's signal handler.
+  int handler_flags = chains[signo].action_.sa_flags;
+  ucontext_t* ucontext = static_cast<ucontext_t*>(ucontext_raw);
+  sigset_t mask;
+  sigorset(&mask, &ucontext->uc_sigmask, &chains[signo].action_.sa_mask);
+  if ((handler_flags & SA_NODEFER)) {
+    sigdelset(&mask, signo);
+  }
+  sigprocmask(SIG_SETMASK, &mask, nullptr);
+
+  if ((handler_flags & SA_SIGINFO)) {
+    chains[signo].action_.sa_sigaction(signo, siginfo, ucontext_raw);
   } else {
-    if (action.sa_sigaction != nullptr) {
-      sigset_t old_mask;
-      sigprocmask(SIG_BLOCK, &action.sa_mask, &old_mask);
-      action.sa_sigaction(sig, info, context);
-      sigprocmask(SIG_SETMASK, &old_mask, nullptr);
+    auto handler = chains[signo].action_.sa_handler;
+    if (handler == SIG_IGN) {
+      return;
+    } else if (handler == SIG_DFL) {
+      raise(signo);
     } else {
-      signal(sig, SIG_DFL);
-      raise(sig);
+      handler(signo);
     }
   }
 }
 
-extern "C" void EnsureFrontOfChain(int signal, struct sigaction* expected_action) {
-  CheckSignalValid(signal);
-  // Read the current action without looking at the chain, it should be the expected action.
-  SigActionFnPtr linked_sigaction = reinterpret_cast<SigActionFnPtr>(linked_sigaction_sym);
-  struct sigaction current_action;
-  linked_sigaction(signal, nullptr, &current_action);
-  // If the sigactions don't match then we put the current action on the chain and make ourself as
-  // the main action.
-  if (current_action.sa_sigaction != expected_action->sa_sigaction) {
-    log("Warning: Unexpected sigaction action found %p\n", current_action.sa_sigaction);
-    user_sigactions[signal].Claim(current_action);
-    linked_sigaction(signal, expected_action, nullptr);
-  }
-}
-
 extern "C" int sigaction(int signal, const struct sigaction* new_action, struct sigaction* old_action) {
   // If this signal has been claimed as a signal chain, record the user's
   // action but don't pass it on to the kernel.
   // Note that we check that the signal number is in range here.  An out of range signal
   // number should behave exactly as the libc sigaction.
-  if (signal > 0 && signal < _NSIG && user_sigactions[signal].IsClaimed() &&
-      (new_action == nullptr || new_action->sa_handler != SIG_DFL)) {
-    struct sigaction saved_action = user_sigactions[signal].GetAction();
+  if (signal < 0 || signal >= _NSIG) {
+    errno = EINVAL;
+    return -1;
+  }
+
+  if (chains[signal].IsClaimed() && new_action != nullptr) {
+    struct sigaction saved_action = chains[signal].GetAction();
     if (new_action != nullptr) {
-      user_sigactions[signal].SetAction(*new_action, false);
+      chains[signal].SetAction(new_action);
     }
     if (old_action != nullptr) {
       *old_action = saved_action;
@@ -216,69 +256,43 @@
 
   // Will only get here if the signal chain has not been claimed.  We want
   // to pass the sigaction on to the kernel via the real sigaction in libc.
-
-  if (linked_sigaction_sym == nullptr) {
-    // Perform lazy initialization.
-    // This will only occur outside of a signal context since we have
-    // not been initialized and therefore cannot be within the ART
-    // runtime.
-    InitializeSignalChain();
-  }
-
-  if (linked_sigaction_sym == nullptr) {
-    log("Unable to find next sigaction in signal chain");
-    abort();
-  }
-  SigActionFnPtr linked_sigaction = reinterpret_cast<SigActionFnPtr>(linked_sigaction_sym);
+  InitializeSignalChain();
   return linked_sigaction(signal, new_action, old_action);
 }
 
-static sighandler_t signal_impl(int signal, sighandler_t handler) {
-  struct sigaction sa;
+extern "C" sighandler_t signal(int signo, sighandler_t handler) {
+  if (signo < 0 || signo > _NSIG) {
+    errno = EINVAL;
+    return SIG_ERR;
+  }
+
+  struct sigaction sa = {};
   sigemptyset(&sa.sa_mask);
   sa.sa_handler = handler;
-  sa.sa_flags = SA_RESTART;
+  sa.sa_flags = SA_RESTART | SA_ONSTACK;
   sighandler_t oldhandler;
 
   // If this signal has been claimed as a signal chain, record the user's
   // action but don't pass it on to the kernel.
-  // Note that we check that the signal number is in range here.  An out of range signal
-  // number should behave exactly as the libc sigaction.
-  if (signal > 0 && signal < _NSIG && user_sigactions[signal].IsClaimed() && handler != SIG_DFL) {
-    oldhandler = reinterpret_cast<sighandler_t>(user_sigactions[signal].GetAction().sa_handler);
-    user_sigactions[signal].SetAction(sa, true);
+  if (chains[signo].IsClaimed()) {
+    oldhandler = reinterpret_cast<sighandler_t>(chains[signo].GetAction().sa_handler);
+    chains[signo].SetAction(&sa);
     return oldhandler;
   }
 
   // Will only get here if the signal chain has not been claimed.  We want
   // to pass the sigaction on to the kernel via the real sigaction in libc.
-
-  if (linked_sigaction_sym == nullptr) {
-    // Perform lazy initialization.
-    InitializeSignalChain();
-  }
-
-  if (linked_sigaction_sym == nullptr) {
-    log("Unable to find next sigaction in signal chain");
-    abort();
-  }
-
-  typedef int (*SigAction)(int, const struct sigaction*, struct sigaction*);
-  SigAction linked_sigaction = reinterpret_cast<SigAction>(linked_sigaction_sym);
-  if (linked_sigaction(signal, &sa, &sa) == -1) {
+  InitializeSignalChain();
+  if (linked_sigaction(signo, &sa, &sa) == -1) {
     return SIG_ERR;
   }
 
   return reinterpret_cast<sighandler_t>(sa.sa_handler);
 }
 
-extern "C" sighandler_t signal(int signal, sighandler_t handler) {
-  return signal_impl(signal, handler);
-}
-
 #if !defined(__LP64__)
-extern "C" sighandler_t bsd_signal(int signal, sighandler_t handler) {
-  return signal_impl(signal, handler);
+extern "C" sighandler_t bsd_signal(int signo, sighandler_t handler) {
+  return signal(signo, handler);
 }
 #endif
 
@@ -292,7 +306,7 @@
       // Don't allow claimed signals in the mask.  If a signal chain has been claimed
       // we can't allow the user to block that signal.
       for (int i = 0 ; i < _NSIG; ++i) {
-        if (user_sigactions[i].IsClaimed() && sigismember(&tmpset, i)) {
+        if (chains[i].IsClaimed() && sigismember(&tmpset, i)) {
           sigdelset(&tmpset, i);
         }
       }
@@ -300,18 +314,7 @@
     new_set_ptr = &tmpset;
   }
 
-  if (linked_sigprocmask_sym == nullptr) {
-    // Perform lazy initialization.
-    InitializeSignalChain();
-  }
-
-  if (linked_sigprocmask_sym == nullptr) {
-    log("Unable to find next sigprocmask in signal chain");
-    abort();
-  }
-
-  typedef int (*SigProcMask)(int how, const sigset_t*, sigset_t*);
-  SigProcMask linked_sigprocmask= reinterpret_cast<SigProcMask>(linked_sigprocmask_sym);
+  InitializeSignalChain();
   return linked_sigprocmask(how, new_set_ptr, bionic_old_set);
 }
 
@@ -322,49 +325,67 @@
   // taken and if it so happens that a signal occurs while one of these
   // locks is already taken, dlsym will block trying to reenter a
   // mutex and we will never get out of it.
+  static bool initialized = false;
   if (initialized) {
     // Don't initialize twice.
     return;
   }
-  linked_sigaction_sym = dlsym(RTLD_NEXT, "sigaction");
+
+  void* linked_sigaction_sym = dlsym(RTLD_NEXT, "sigaction");
   if (linked_sigaction_sym == nullptr) {
     linked_sigaction_sym = dlsym(RTLD_DEFAULT, "sigaction");
     if (linked_sigaction_sym == nullptr ||
         linked_sigaction_sym == reinterpret_cast<void*>(sigaction)) {
-      linked_sigaction_sym = nullptr;
+      fatal("Unable to find next sigaction in signal chain");
     }
   }
 
-  linked_sigprocmask_sym = dlsym(RTLD_NEXT, "sigprocmask");
+  void* linked_sigprocmask_sym = dlsym(RTLD_NEXT, "sigprocmask");
   if (linked_sigprocmask_sym == nullptr) {
     linked_sigprocmask_sym = dlsym(RTLD_DEFAULT, "sigprocmask");
     if (linked_sigprocmask_sym == nullptr ||
         linked_sigprocmask_sym == reinterpret_cast<void*>(sigprocmask)) {
-      linked_sigprocmask_sym = nullptr;
+      fatal("Unable to find next sigprocmask in signal chain");
     }
   }
+
+  linked_sigaction = reinterpret_cast<decltype(linked_sigaction)>(linked_sigaction_sym);
+  linked_sigprocmask = reinterpret_cast<decltype(linked_sigprocmask)>(linked_sigprocmask_sym);
   initialized = true;
 }
 
-extern "C" void SetSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn) {
-  CheckSignalValid(signal);
+extern "C" void AddSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn) {
+  if (signal <= 0 || signal >= _NSIG) {
+    fatal("Invalid signal %d", signal);
+  }
 
   // Set the managed_handler.
-  user_sigactions[signal].SetSpecialHandler(fn);
+  chains[signal].AddSpecialHandler(fn);
+  chains[signal].Claim(signal);
+}
 
-  // In case the chain isn't claimed, claim it for ourself so we can ensure the managed handler
-  // goes first.
-  if (!user_sigactions[signal].IsClaimed()) {
-    struct sigaction act, old_act;
-    act.sa_sigaction = sigchainlib_managed_handler_sigaction;
-    sigemptyset(&act.sa_mask);
-    act.sa_flags = SA_SIGINFO | SA_ONSTACK;
-#if !defined(__APPLE__) && !defined(__mips__)
-    act.sa_restorer = nullptr;
-#endif
-    if (sigaction(signal, &act, &old_act) != -1) {
-      user_sigactions[signal].Claim(old_act);
-    }
+extern "C" void RemoveSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn) {
+  if (signal <= 0 || signal >= _NSIG) {
+    fatal("Invalid signal %d", signal);
+  }
+
+  chains[signal].RemoveSpecialHandler(fn);
+}
+
+extern "C" void EnsureFrontOfChain(int signal) {
+  if (signal <= 0 || signal >= _NSIG) {
+    fatal("Invalid signal %d", signal);
+  }
+
+  // Read the current action without looking at the chain, it should be the expected action.
+  struct sigaction current_action;
+  InitializeSignalChain();
+  linked_sigaction(signal, nullptr, &current_action);
+  // If the sigactions don't match then we put the current action on the chain and make ourself as
+  // the main action.
+  if (current_action.sa_sigaction != SignalChain::Handler) {
+    log("Warning: Unexpected sigaction action found %p\n", current_action.sa_sigaction);
+    chains[signal].Register(signal);
   }
 }
 
diff --git a/sigchainlib/sigchain.h b/sigchainlib/sigchain.h
index 01ccedf..960d221 100644
--- a/sigchainlib/sigchain.h
+++ b/sigchainlib/sigchain.h
@@ -23,16 +23,11 @@
 
 extern "C" void InitializeSignalChain();
 
-extern "C" void ClaimSignalChain(int signal, struct sigaction* oldaction);
-
-extern "C" void UnclaimSignalChain(int signal);
-
 typedef bool (*SpecialSignalHandlerFn)(int, siginfo_t*, void*);
-extern "C" void SetSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn);
+extern "C" void AddSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn);
+extern "C" void RemoveSpecialSignalHandlerFn(int signal, SpecialSignalHandlerFn fn);
 
-extern "C" void InvokeUserSignalHandler(int sig, siginfo_t* info, void* context);
-
-extern "C" void EnsureFrontOfChain(int signal, struct sigaction* expected_action);
+extern "C" void EnsureFrontOfChain(int signal);
 
 }  // namespace art
 
diff --git a/sigchainlib/sigchain_dummy.cc b/sigchainlib/sigchain_dummy.cc
index aa3c360..d6a5e12 100644
--- a/sigchainlib/sigchain_dummy.cc
+++ b/sigchainlib/sigchain_dummy.cc
@@ -48,37 +48,23 @@
 
 namespace art {
 
-
-extern "C" void ClaimSignalChain(int signal ATTRIBUTE_UNUSED,
-                                 struct sigaction* oldaction ATTRIBUTE_UNUSED) {
-  log("ClaimSignalChain is not exported by the main executable.");
-  abort();
-}
-
-extern "C" void UnclaimSignalChain(int signal ATTRIBUTE_UNUSED) {
-  log("UnclaimSignalChain is not exported by the main executable.");
-  abort();
-}
-
-extern "C" void InvokeUserSignalHandler(int sig ATTRIBUTE_UNUSED,
-                                        siginfo_t* info ATTRIBUTE_UNUSED,
-                                        void* context ATTRIBUTE_UNUSED) {
-  log("InvokeUserSignalHandler is not exported by the main executable.");
-  abort();
-}
-
 extern "C" void InitializeSignalChain() {
   log("InitializeSignalChain is not exported by the main executable.");
   abort();
 }
 
-extern "C" void EnsureFrontOfChain(int signal ATTRIBUTE_UNUSED,
-                                   struct sigaction* expected_action ATTRIBUTE_UNUSED) {
+extern "C" void EnsureFrontOfChain(int signal ATTRIBUTE_UNUSED) {
   log("EnsureFrontOfChain is not exported by the main executable.");
   abort();
 }
 
-extern "C" void SetSpecialSignalHandlerFn(int signal ATTRIBUTE_UNUSED,
+extern "C" void AddSpecialSignalHandlerFn(int signal ATTRIBUTE_UNUSED,
+                                          SpecialSignalHandlerFn fn ATTRIBUTE_UNUSED) {
+  log("SetSpecialSignalHandlerFn is not exported by the main executable.");
+  abort();
+}
+
+extern "C" void RemoveSpecialSignalHandlerFn(int signal ATTRIBUTE_UNUSED,
                                           SpecialSignalHandlerFn fn ATTRIBUTE_UNUSED) {
   log("SetSpecialSignalHandlerFn is not exported by the main executable.");
   abort();
diff --git a/sigchainlib/version-script32.txt b/sigchainlib/version-script32.txt
index eec9103..f360efa 100644
--- a/sigchainlib/version-script32.txt
+++ b/sigchainlib/version-script32.txt
@@ -1,11 +1,9 @@
 {
 global:
-  ClaimSignalChain;
-  UnclaimSignalChain;
-  InvokeUserSignalHandler;
   InitializeSignalChain;
   EnsureFrontOfChain;
-  SetSpecialSignalHandlerFn;
+  AddSpecialSignalHandlerFn;
+  RemoveSpecialSignalHandlerFn;
   bsd_signal;
   sigaction;
   signal;
diff --git a/sigchainlib/version-script64.txt b/sigchainlib/version-script64.txt
index 08c312e..319d1c6 100644
--- a/sigchainlib/version-script64.txt
+++ b/sigchainlib/version-script64.txt
@@ -1,11 +1,9 @@
 {
 global:
-  ClaimSignalChain;
-  UnclaimSignalChain;
-  InvokeUserSignalHandler;
   InitializeSignalChain;
   EnsureFrontOfChain;
-  SetSpecialSignalHandlerFn;
+  AddSpecialSignalHandlerFn;
+  RemoveSpecialSignalHandlerFn;
   sigaction;
   signal;
   sigprocmask;
diff --git a/test/004-SignalTest/expected.txt b/test/004-SignalTest/expected.txt
index b3a0e1c..847b56f 100644
--- a/test/004-SignalTest/expected.txt
+++ b/test/004-SignalTest/expected.txt
@@ -3,4 +3,8 @@
 Caught NullPointerException
 Caught StackOverflowError
 signal caught
+unblocked signal received
+unblocking blocked signal
+blocked signal received
+signal handler done
 Signal test OK
diff --git a/test/004-SignalTest/signaltest.cc b/test/004-SignalTest/signaltest.cc
index 6dd6355..a115c0e 100644
--- a/test/004-SignalTest/signaltest.cc
+++ b/test/004-SignalTest/signaltest.cc
@@ -18,13 +18,14 @@
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/ucontext.h>
 #include <unistd.h>
 
 #include "base/macros.h"
 
 static int signal_count;
-static const int kMaxSignal = 2;
+static const int kMaxSignal = 1;
 
 #if defined(__i386__) || defined(__x86_64__)
 #if defined(__APPLE__)
@@ -47,6 +48,17 @@
 #endif
 #endif
 
+#define BLOCKED_SIGNAL SIGUSR1
+#define UNBLOCKED_SIGNAL SIGUSR2
+
+static void blocked_signal(int sig ATTRIBUTE_UNUSED) {
+  printf("blocked signal received\n");
+}
+
+static void unblocked_signal(int sig ATTRIBUTE_UNUSED) {
+  printf("unblocked signal received\n");
+}
+
 static void signalhandler(int sig ATTRIBUTE_UNUSED, siginfo_t* info ATTRIBUTE_UNUSED,
                           void* context) {
   printf("signal caught\n");
@@ -54,6 +66,16 @@
   if (signal_count > kMaxSignal) {
      abort();
   }
+
+  raise(UNBLOCKED_SIGNAL);
+  raise(BLOCKED_SIGNAL);
+  printf("unblocking blocked signal\n");
+
+  sigset_t mask;
+  sigemptyset(&mask);
+  sigaddset(&mask, BLOCKED_SIGNAL);
+  sigprocmask(SIG_UNBLOCK, &mask, nullptr);
+
 #if defined(__arm__)
   struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
   struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
@@ -71,6 +93,8 @@
 #else
   UNUSED(context);
 #endif
+
+  printf("signal handler done\n");
 }
 
 static struct sigaction oldaction;
@@ -78,13 +102,16 @@
 extern "C" JNIEXPORT void JNICALL Java_Main_initSignalTest(JNIEnv*, jclass) {
   struct sigaction action;
   action.sa_sigaction = signalhandler;
-  sigemptyset(&action.sa_mask);
+  sigfillset(&action.sa_mask);
+  sigdelset(&action.sa_mask, UNBLOCKED_SIGNAL);
   action.sa_flags = SA_SIGINFO | SA_ONSTACK;
 #if !defined(__APPLE__) && !defined(__mips__)
   action.sa_restorer = nullptr;
 #endif
 
   sigaction(SIGSEGV, &action, &oldaction);
+  signal(BLOCKED_SIGNAL, blocked_signal);
+  signal(UNBLOCKED_SIGNAL, unblocked_signal);
 }
 
 extern "C" JNIEXPORT void JNICALL Java_Main_terminateSignalTest(JNIEnv*, jclass) {
@@ -96,6 +123,12 @@
 char *go_away_compiler = nullptr;
 
 extern "C" JNIEXPORT jint JNICALL Java_Main_testSignal(JNIEnv*, jclass) {
+  // Unblock UNBLOCKED_SIGNAL.
+  sigset_t mask;
+  memset(&mask, 0, sizeof(mask));
+  sigaddset(&mask, UNBLOCKED_SIGNAL);
+  sigprocmask(SIG_UNBLOCK, &mask, nullptr);
+
 #if defined(__arm__) || defined(__i386__) || defined(__aarch64__)
   // On supported architectures we cause a real SEGV.
   *go_away_compiler = 'a';