Merge pull request #1353 from sporksmith/disable-forkproxy

Disable forkproxy by default

* Disable forkproxy / O(n)-waitpid-workaround by default
* Fix a shutdown bug in thread_ptrace that this exposed
    
Closes #1261

docs/3.1-Shadow-Config.md

@@ -260,10 +260,10 @@ Use the MemoryManager. It can be useful to disable for debugging, but will hurt
 
 #### `experimental.use_o_n_waitpid_workarounds`
 
-Default: true  
+Default: false
 Type: Bool
 
-Enable performance workarounds for waitpid being O(n). Beneficial to disable if waitpid is patched to be O(1) or in some cases where it'd otherwise result in excessive detaching and reattaching.
+Use performance workarounds for waitpid being O(n). Beneficial to disable if waitpid is patched to be O(1), if using one logical processor per host, or in some cases where it'd otherwise result in excessive detaching and reattaching.
 
 #### `experimental.use_object_counters`
 

src/main/core/support/configuration.rs

@@ -194,9 +194,9 @@ pub struct ExperimentalOptions {
     #[clap(about = EXP_HELP.get("use_sched_fifo").unwrap())]
     use_sched_fifo: Option<bool>,
 
-    /// Enable performance workarounds for waitpid being O(n). Beneficial to disable if waitpid
-    /// is patched to be O(1) or in some cases where it'd otherwise result in excessive detaching
-    /// and reattaching
+    /// Use performance workarounds for waitpid being O(n). Beneficial to disable if waitpid
+    /// is patched to be O(1), if using one logical processor per host, or in some cases where
+    /// it'd otherwise result in excessive detaching and reattaching
     #[clap(long, value_name = "bool")]
     #[clap(about = EXP_HELP.get("use_o_n_waitpid_workarounds").unwrap())]
     use_o_n_waitpid_workarounds: Option<bool>,
@@ -300,7 +300,7 @@ impl Default for ExperimentalOptions {
     fn default() -> Self {
         Self {
             use_sched_fifo: Some(false),
-            use_o_n_waitpid_workarounds: Some(true),
+            use_o_n_waitpid_workarounds: Some(false),
             use_explicit_block_message: Some(false),
             use_syscall_counters: Some(false),
             use_object_counters: Some(true),

src/main/host/thread_ptrace.c

@@ -1098,29 +1098,32 @@ void threadptrace_handleProcessExit(Thread* base) {
     trace("handleProcessExit for thread %d.%d native %d.%d", thread_getProcessId(&thread->base),
           thread_getID(&thread->base), thread_getNativePid(&thread->base),
           thread_getNativeTid(&thread->base));
+
     if (!thread_isRunning(base)) {
         // Nothing to do
         utility_assert(!thread->sys);
         return;
     }
 
-    // Try to catch exit event. Exact conditions under which we need to do this
-    // are unclear, but detaching sometimes fails if we don't.
-    int wstatus;
-    pid_t pid = waitpid(thread->base.nativeTid, &wstatus, WAITPID_COMMON_OPTIONS);
-    if (pid < 0) {
-        if (errno == ECHILD) {
-            // Don't fully understand when this happens or not. Experimentally
-            // we *do* still need to continue to detach even if this is the
-            // case.
-            trace("Couldn't wait on dying child thread disappeared");
+    if (!thread->needAttachment) {
+        // Try to catch exit event. Exact conditions under which we need to do this
+        // are unclear, but detaching sometimes fails if we don't.
+        int wstatus;
+        pid_t pid = waitpid(thread->base.nativeTid, &wstatus, WAITPID_COMMON_OPTIONS);
+        if (pid < 0) {
+            if (errno == ECHILD) {
+                // Don't fully understand when this happens or not. Experimentally
+                // we *do* still need to continue to detach even if this is the
+                // case.
+                trace("Couldn't wait on dying child thread disappeared");
+            } else {
+                warning("Unexpected waitpid: %s", strerror(errno));
+            }
         } else {
-            warning("Unexpected waitpid: %s", strerror(errno));
-        }
-    } else {
-        StopReason ptraceStopReason = _getStopReason(wstatus);
-        if (ptraceStopReason.type != STOPREASON_EXIT_EVENT) {
-            warning("Unexpected stop reason type %d", ptraceStopReason.type);
+            StopReason ptraceStopReason = _getStopReason(wstatus);
+            if (ptraceStopReason.type != STOPREASON_EXIT_EVENT) {
+                warning("Unexpected stop reason type %d", ptraceStopReason.type);
+            }
         }
     }