tor: stop racing the watchdog for the managed-Tor pid

BtfStopManagedTor read, waited on and signalled g_managedTorPid without
holding cs_managedTor, while the watchdog concurrently reaps the same child
and resets it. If the watchdog won, the stop path spun ten seconds on ECHILD
and then sent SIGKILL to a stale, potentially recycled pid. Snapshot the pid
under the lock, treat ECHILD as already-exited, and only clear the global if
it still holds our pid.

1 files changed, 18 insertions, 7 deletions

diff --git a/src/tor.cpp b/src/tor.cpp
index 20b4577..fea6912 100644
--- a/src/tor.cpp
+++ b/src/tor.cpp
@@ -827,17 +827,26 @@ void BtfStopManagedTor()
         memset(&g_managedTorProcess, 0, sizeof(g_managedTorProcess));
     }
 #else
-    if (g_managedTorPid > 0)
+    // Snapshot the pid under the lock. The watchdog's ManagedTorProcessAlive
+    // reaps the same child concurrently and resets g_managedTorPid under
+    // cs_managedTor; reading the global unlocked here could leave us waiting
+    // on ECHILD and then SIGKILLing a recycled PID.
+    pid_t pid = -1;
+    CRITICAL_BLOCK(cs_managedTor)
+        pid = g_managedTorPid;
+    if (pid > 0)
     {
         std::string shutdownErr;
         if (!RequestManagedTorShutdown(shutdownErr))
-            kill(g_managedTorPid, SIGTERM);
+            kill(pid, SIGTERM);
         bool fExited = false;
         for (int i = 0; i < 100; i++)
         {
             int status = 0;
-            pid_t r = waitpid(g_managedTorPid, &status, WNOHANG);
-            if (r == g_managedTorPid)
+            pid_t r = waitpid(pid, &status, WNOHANG);
+            // ECHILD means the watchdog already reaped it -- that is exit,
+            // not a reason to signal anything.
+            if (r == pid || (r == -1 && errno == ECHILD))
             {
                 fExited = true;
                 break;
@@ -846,10 +855,12 @@ void BtfStopManagedTor()
         }
         if (!fExited)
         {
-            kill(g_managedTorPid, SIGKILL);
-            waitpid(g_managedTorPid, NULL, 0);
+            kill(pid, SIGKILL);
+            waitpid(pid, NULL, 0);
         }
-        g_managedTorPid = -1;
+        CRITICAL_BLOCK(cs_managedTor)
+            if (g_managedTorPid == pid)
+                g_managedTorPid = -1;
     }
 #endif
     CRITICAL_BLOCK(cs_managedTor)