server restart stuck fixes

server/install_server.sh

@@ -158,6 +158,7 @@ Environment=REPLAY_SEGMENT_DIR=$REPLAY_SEGMENT_DIR
 ExecStart=$NODE_BIN $SERVER_DIR/index.js
 Restart=on-failure
 RestartSec=2
+SuccessExitStatus=130 143
 
 [Install]
 WantedBy=multi-user.target

server/src/services/audioForwardService/hooks.js

@@ -31,15 +31,13 @@ function registerAudioForwardHooks(deps) {
       stopWorker(roverId);
       return;
     }
-    if (action === 'upsert' && serviceEnabled) {
-      if (whipOwners.has(roverId)) {
-        return;
-      }
-      try {
-        ensureWorker(roverId);
-      } catch (err) {
-        setState(roverId, { state: 'error', source: 'init', error: err.message, startedAt: null });
-      }
+    if (action === 'upsert' && serviceEnabled && !workers.has(roverId)) {
+      // A rover coming online should not create ffmpeg publishers by itself.
+      // The audio worker is intentionally lazy because uploads, mic forwarding,
+      // and automatic sounds are the moments that actually need a media pipe;
+      // keeping idle ffmpeg children around made restarts depend on processes
+      // that may never have been used by an operator.
+      setState(roverId, { state: 'offline', source: 'none', error: null, startedAt: null });
     }
   });
 

server/src/services/audioForwardService/index.js

@@ -95,6 +95,7 @@ const workerEngine = createAudioForwardWorkerEngine({
 const {
   ensureWorker,
   stopWorker,
+  stopAllWorkers,
   playUploadedAudio,
   playServerAudioFile,
   stopPlayback,
@@ -104,6 +105,21 @@ const {
   startSilenceWriter,
 } = workerEngine;
 
+function installShutdownHooks() {
+  const shutdown = (signal) => {
+    // Audio forwarding owns long-lived ffmpeg publisher/writer pairs. Stop them
+    // synchronously on process signals so a systemd restart does not have to
+    // wait for orphaned media workers to notice that their parent is gone.
+    stopAllWorkers(signal || 'process-exit');
+  };
+
+  process.once('exit', () => shutdown('exit'));
+  process.once('SIGINT', () => shutdown('SIGINT'));
+  process.once('SIGTERM', () => shutdown('SIGTERM'));
+}
+
+installShutdownHooks();
+
 registerAudioForwardHooks({
   io,
   roverManager,

server/src/services/audioForwardService/workerEngine.js

@@ -80,14 +80,26 @@ function createAudioForwardWorkerEngine(deps) {
   }
 
   function stopProc(proc, graceMs = 1200) {
-    if (!proc || proc.killed) return;
+    if (!proc || proc.exitCode != null || proc.signalCode != null) return;
+    let exited = false;
+    const markExited = () => {
+      exited = true;
+    };
+    // ChildProcess.killed only means Node successfully sent a signal, not that
+    // ffmpeg actually exited. Track the real exit event so FIFO/SRT hangs still
+    // get escalated to SIGKILL instead of making systemd wait for its timeout.
+    proc.once('exit', markExited);
     try {
       proc.kill('SIGTERM');
     } catch {
+      proc.off('exit', markExited);
       return;
     }
     setTimeout(() => {
-      if (!proc.killed) {
+      // The timer intentionally checks our exit flag rather than proc.killed.
+      // proc.killed flips to true immediately after SIGTERM, which was the bug
+      // that prevented stubborn ffmpeg processes from being force-killed.
+      if (!exited) {
         try {
           proc.kill('SIGKILL');
         } catch {
@@ -394,6 +406,19 @@ function createAudioForwardWorkerEngine(deps) {
     setState(roverId, { state: 'offline', source: 'none', error: null, startedAt: null });
   }
 
+  function stopAllWorkers(reason = 'shutdown') {
+    // Copy the keys before stopping because stopWorker mutates the workers map.
+    // Shutdown is a process-wide lifecycle event, so every rover-owned ffmpeg
+    // process must be asked to exit before the parent Node process disappears.
+    const roverIds = Array.from(workers.keys());
+    if (roverIds.length) {
+      logger.info('Stopping all audio forward workers', { reason, roverIds });
+    }
+    roverIds.forEach((roverId) => {
+      stopWorker(roverId);
+    });
+  }
+
   function writeUploadFile(roverId, payload = {}) {
     const { name, mime, dataBase64 } = payload || {};
     const ext = extFromUpload(name, mime);
@@ -497,6 +522,7 @@ function createAudioForwardWorkerEngine(deps) {
   return {
     ensureWorker,
     stopWorker,
+    stopAllWorkers,
     playUploadedAudio,
     playServerAudioFile,
     stopPlayback,

server/src/services/kinectService/hardware.js

@@ -8,6 +8,7 @@ const sharp = require('sharp');
 const WORKER_PATH = process.env.KINECT_WORKER || path.join(__dirname, 'native', 'kinect_worker');
 const CAPTURE_TIMEOUT_MS = 12000;
 const WORKER_STDERR_LOG_INTERVAL_MS = 5000;
+const PROCESS_SIGNAL_EXIT_DELAY_MS = 1700;
 
 let worker = null;
 let stdoutBuffer = Buffer.alloc(0);
@@ -230,14 +231,23 @@ async function getWorkerStatus() {
 
 function installShutdownHooks() {
   const shutdown = () => stopWorker();
+  const exitAfterServiceCleanup = (code) => {
+    // Several services install signal handlers that synchronously start cleanup
+    // and schedule short force-kill fallbacks for child processes. Exiting here
+    // immediately would prevent those timers from running, so the Kinect bridge
+    // leaves a small process-wide grace window before forcing Node down.
+    setTimeout(() => {
+      process.exit(code);
+    }, PROCESS_SIGNAL_EXIT_DELAY_MS);
+  };
   process.once('exit', shutdown);
   process.once('SIGINT', () => {
     shutdown();
-    process.exit(130);
+    exitAfterServiceCleanup(130);
   });
   process.once('SIGTERM', () => {
     shutdown();
-    process.exit(143);
+    exitAfterServiceCleanup(143);
   });
 }