Simplify the signalhandlers module (#854)

* Remove the unused EnsureDarwinMachPorts function

When compiling the C++ shims for longjmp/setjmp/signal handling we don't
use the `USE_APPLE_MACH_PORTS` directive, so this function was entirely
unused anyway. This looks to be a holdover from when this was originally
copied from C++, but no need for keeping around this now-legacy
initialization.

* Remove the `wasmtime_init_finish` function

This looks like it's perhaps largely historical cruft at this point now
I think? The function, with the removal of the mach ports from the
previous commit, only reads the initializtion state of the signal
handlers. If the signal handlers failed to get installed, though, it
simply returns early rather than erroring out anyway. In any case a
follow-up commit will refactor `wasmtime_init_eager` to handle this as
well.

* Pare down `wasmtime_init_eager`

Similar to previous commits it looks like this function may have accrued
some debt over time, nowadays it doesn't really do much other than
capture a backtrace and install signal handlers. The `lazy_static` state
isn't really that necessary and we can rely on the `Once` primitive in
the standard library for one-time initialization.

This also updates the code to unconditionally panic if signal handlers
fail to get installed, which I think is the behavior that we'll want for
now and we can enhance it over time if necessary, but I don't think we
have a use case where it's currently necessary.

crates/runtime/signalhandlers/SignalHandlers.cpp

@@ -779,59 +779,3 @@ EnsureEagerSignalHandlers()
 
     return true;
 }
-
-int
-EnsureDarwinMachPorts()
-{
-#ifdef USE_APPLE_MACH_PORTS
-    pthread_attr_t handlerThreadAttr;
-    int r = pthread_attr_init(&handlerThreadAttr);
-    if (r != 0) {
-        return false;
-    }
-
-    // Create the port that all of our threads will redirect their traps to.
-    kern_return_t kret;
-    kret = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &sMachDebugPort);
-    if (kret != KERN_SUCCESS) {
-        return false;
-    }
-    kret = mach_port_insert_right(mach_task_self(), sMachDebugPort, sMachDebugPort,
-                                  MACH_MSG_TYPE_MAKE_SEND);
-    if (kret != KERN_SUCCESS) {
-        return false;
-    }
-
-    // Create the thread that will wait on and service sMachDebugPort.
-    // It's not useful to destroy this thread on process shutdown so
-    // immediately detach on successful start.
-    pthread_t handlerThread;
-    r = pthread_create(&handlerThread, &handlerThreadAttr, MachExceptionHandlerThread, nullptr);
-    if (r != 0) {
-        return false;
-    }
-    r = pthread_detach(handlerThread);
-    assert(r == 0);
-
-    // In addition to the process-wide signal handler setup, OSX needs each
-    // thread configured to send its exceptions to sMachDebugPort. While there
-    // are also task-level (i.e. process-level) exception ports, those are
-    // "claimed" by breakpad and chaining Mach exceptions is dark magic that we
-    // avoid by instead intercepting exceptions at the thread level before they
-    // propagate to the process-level. This works because there are no other
-    // uses of thread-level exception ports.
-    assert(sMachDebugPort != MACH_PORT_NULL);
-    thread_port_t thisThread = mach_thread_self();
-    kret = thread_set_exception_ports(thisThread,
-                                                    EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION,
-                                                    sMachDebugPort,
-                                                    EXCEPTION_DEFAULT | MACH_EXCEPTION_CODES,
-                                                    THREAD_STATE_NONE);
-    mach_port_deallocate(mach_task_self(), thisThread);
-    if (kret != KERN_SUCCESS) {
-        return false;
-    }
-
-#endif
-    return true;
-}

crates/runtime/signalhandlers/SignalHandlers.hpp

@@ -40,13 +40,6 @@ void Unwind(void*);
 int
 EnsureEagerSignalHandlers(void);
 
-// Assuming EnsureEagerProcessSignalHandlers() has already been called,
-// this function performs the full installation of signal handlers which must
-// be performed per-thread. This operation may incur some overhead and
-// so should be done only when needed to use wasm.
-int
-EnsureDarwinMachPorts(void);
-
 #ifdef __cplusplus
 } // extern "C"
 #endif

crates/runtime/src/instance.rs

@@ -7,7 +7,7 @@ use crate::imports::Imports;
 use crate::jit_int::GdbJitImageRegistration;
 use crate::memory::LinearMemory;
 use crate::mmap::Mmap;
-use crate::signalhandlers::{wasmtime_init_eager, wasmtime_init_finish};
+use crate::signalhandlers;
 use crate::table::Table;
 use crate::traphandlers::{wasmtime_call, Trap};
 use crate::vmcontext::{
@@ -611,14 +611,6 @@ impl Instance {
         index
     }
 
-    /// Test whether any of the objects inside this instance require signal
-    /// handlers to catch out of bounds accesses.
-    pub(crate) fn needs_signal_handlers(&self) -> bool {
-        self.memories
-            .values()
-            .any(|memory| memory.needs_signal_handlers)
-    }
-
     /// Grow memory by the specified amount of pages.
     ///
     /// Returns `None` if memory can't be grown by the specified amount
@@ -871,8 +863,7 @@ impl InstanceHandle {
 
         // Ensure that our signal handlers are ready for action.
         // TODO: Move these calls out of `InstanceHandle`.
-        wasmtime_init_eager();
+        signalhandlers::init();
-        wasmtime_init_finish(instance.vmctx_mut());
 
         // The WebAssembly spec specifies that the start function is
         // invoked automatically at instantiation time.

crates/runtime/src/lib.rs

@@ -43,7 +43,6 @@ pub use crate::instance::{InstanceHandle, InstantiationError, LinkError};
 pub use crate::jit_int::GdbJitImageRegistration;
 pub use crate::mmap::Mmap;
 pub use crate::sig_registry::SignatureRegistry;
-pub use crate::signalhandlers::{wasmtime_init_eager, wasmtime_init_finish};
 pub use crate::trap_registry::{get_mut_trap_registry, get_trap_registry, TrapRegistrationGuard};
 pub use crate::traphandlers::{wasmtime_call, wasmtime_call_trampoline, Trap};
 pub use crate::vmcontext::{

crates/runtime/src/signalhandlers.rs

@@ -1,66 +1,26 @@
 //! Interface to low-level signal-handling mechanisms.
 
-#![allow(non_upper_case_globals)]
+use std::sync::Once;
-#![allow(non_camel_case_types)]
-#![allow(non_snake_case)]
-
-use crate::vmcontext::VMContext;
-use lazy_static::lazy_static;
-use std::borrow::{Borrow, BorrowMut};
-use std::cell::Cell;
-use std::sync::RwLock;
-
-#[derive(Default)]
-struct TrapContext {
-    tried_to_install_signal_handlers: Cell<bool>,
-    have_signal_handlers: Cell<bool>,
-}
 
 extern "C" {
     fn EnsureEagerSignalHandlers() -> libc::c_int;
-    #[cfg(any(target_os = "macos", target_os = "ios"))]
-    fn EnsureDarwinMachPorts() -> libc::c_int;
 }
 
-struct InstallState {
+/// This function performs the low-overhead signal handler initialization that
-    tried: bool,
+/// we want to do eagerly to ensure a more-deterministic global process state.
-    success: bool,
+///
+/// This is especially relevant for signal handlers since handler ordering
+/// depends on installation order: the wasm signal handler must run *before*
+/// the other crash handlers and since POSIX signal handlers work LIFO, this
+/// function needs to be called at the end of the startup process, after other
+/// handlers have been installed. This function can thus be called multiple
+/// times, having no effect after the first call.
+pub fn init() {
+    static INIT: Once = Once::new();
+    INIT.call_once(real_init);
 }
 
-impl InstallState {
+fn real_init() {
-    fn new() -> Self {
-        Self {
-            tried: false,
-            success: false,
-        }
-    }
-}
-
-lazy_static! {
-    static ref EAGER_INSTALL_STATE: RwLock<InstallState> = RwLock::new(InstallState::new());
-    static ref LAZY_INSTALL_STATE: RwLock<InstallState> = RwLock::new(InstallState::new());
-}
-
-/// This function performs the low-overhead signal handler initialization that we
-/// want to do eagerly to ensure a more-deterministic global process state. This
-/// is especially relevant for signal handlers since handler ordering depends on
-/// installation order: the wasm signal handler must run *before* the other crash
-/// handlers and since POSIX signal handlers work LIFO, this function needs to be
-/// called at the end of the startup process, after other handlers have been
-/// installed. This function can thus be called multiple times, having no effect
-/// after the first call.
-#[no_mangle]
-pub extern "C" fn wasmtime_init_eager() {
-    let mut locked = EAGER_INSTALL_STATE.write().unwrap();
-    let state = locked.borrow_mut();
-
-    if state.tried {
-        return;
-    }
-
-    state.tried = true;
-    assert!(!state.success);
-
     // This is a really weird and unfortunate function call. For all the gory
     // details see #829, but the tl;dr; is that in a trap handler we have 2
     // pages of stack space on Linux, and calling into libunwind which triggers
@@ -73,70 +33,6 @@ pub extern "C" fn wasmtime_init_eager() {
     drop(backtrace::Backtrace::new_unresolved());
 
     if unsafe { EnsureEagerSignalHandlers() == 0 } {
-        return;
-    }
-
-    state.success = true;
-}
-
-thread_local! {
-    static TRAP_CONTEXT: TrapContext = TrapContext::default();
-}
-
-/// Assuming `EnsureEagerProcessSignalHandlers` has already been called,
-/// this function performs the full installation of signal handlers which must
-/// be performed per-thread. This operation may incur some overhead and
-/// so should be done only when needed to use wasm.
-#[no_mangle]
-pub extern "C" fn wasmtime_init_finish(vmctx: &mut VMContext) {
-    if !TRAP_CONTEXT.with(|cx| cx.tried_to_install_signal_handlers.get()) {
-        TRAP_CONTEXT.with(|cx| {
-            cx.tried_to_install_signal_handlers.set(true);
-            assert!(!cx.have_signal_handlers.get());
-        });
-
-        {
-            let locked = EAGER_INSTALL_STATE.read().unwrap();
-            let state = locked.borrow();
-            assert!(
-                state.tried,
-                "call wasmtime_init_eager before calling wasmtime_init_finish"
-            );
-            if !state.success {
-                return;
-            }
-        }
-
-        #[cfg(any(target_os = "macos", target_os = "ios"))]
-        ensure_darwin_mach_ports();
-
-        TRAP_CONTEXT.with(|cx| {
-            cx.have_signal_handlers.set(true);
-        })
-    }
-
-    let instance = unsafe { vmctx.instance() };
-    let have_signal_handlers = TRAP_CONTEXT.with(|cx| cx.have_signal_handlers.get());
-    if !have_signal_handlers && instance.needs_signal_handlers() {
         panic!("failed to install signal handlers");
     }
 }
-
-#[cfg(any(target_os = "macos", target_os = "ios"))]
-fn ensure_darwin_mach_ports() {
-    let mut locked = LAZY_INSTALL_STATE.write().unwrap();
-    let state = locked.borrow_mut();
-
-    if state.tried {
-        return;
-    }
-
-    state.tried = true;
-    assert!(!state.success);
-
-    if unsafe { EnsureDarwinMachPorts() != 0 } {
-        return;
-    }
-
-    state.success = true;
-}