diff --git a/Plugin/unraid/api/dryrun.php b/Plugin/unraid/api/dryrun.php index 551cd7f..449bcea 100644 --- a/Plugin/unraid/api/dryrun.php +++ b/Plugin/unraid/api/dryrun.php @@ -106,6 +106,8 @@ $extraStr = ''; foreach (preg_split('/\s+/', $extra_args, -1, PREG_SPLIT_NO_EMPTY) as $tok) { $extraStr .= ' ' . escapeshellarg($tok); } -exec('nohup bash ' . escapeshellarg($runner) . ' ' . escapeshellarg($id) . ' ' . escapeshellarg($script) . ' --dry-run' . ($flags ? " $flags" : '') . ' --manual' . $locArg . $extraStr . ' >> ' . escapeshellarg($logFile) . ' 2>&1 > ' . escapeshellarg($logFile) . ' 2>&1 true]); diff --git a/Plugin/unraid/api/run.php b/Plugin/unraid/api/run.php index a0d898d..d8e67ef 100644 --- a/Plugin/unraid/api/run.php +++ b/Plugin/unraid/api/run.php @@ -121,6 +121,10 @@ $extraStr = ''; foreach (preg_split('/\s+/', $extra_args, -1, PREG_SPLIT_NO_EMPTY) as $tok) { $extraStr .= ' ' . escapeshellarg($tok); } -exec('nohup bash ' . escapeshellarg($runner) . ' ' . escapeshellarg($id) . ' ' . escapeshellarg($script) . ($flags ? " $flags" : '') . ' --manual' . $locArg . $extraStr . ' >> ' . escapeshellarg($logFile) . ' 2>&1 > ' . escapeshellarg($logFile) . ' 2>&1 true]); diff --git a/Plugin/unraid/api/stop.php b/Plugin/unraid/api/stop.php index bc203e3..a9f8f2a 100644 --- a/Plugin/unraid/api/stop.php +++ b/Plugin/unraid/api/stop.php @@ -16,10 +16,12 @@ // spinner for a job that no longer exists. // // DESIGN PRINCIPLES -// Kills the group, falls back to the tree. -// `ps -o pgid=` resolves the process group; when that fails the fallback is pkill -P -// plus the pid itself. Two strategies, because a job whose runner already exited can -// leave children whose group id is no longer discoverable from the recorded pid. +// Kills the group only when the job owns the group; otherwise the tree. +// `ps -o pgid=` resolves the process group, and it is used only if it equals the recorded +// pid — i.e. the job is the group leader. Otherwise the fallback is pkill -P plus the pid +// itself. Two strategies, because a job whose runner already exited can leave children +// whose group id is no longer discoverable from the recorded pid, and because a job that +// was not started under setsid shares its group with whatever launched it. // // Waits before escalating. // Six 500ms checks between TERM and KILL. Scripts have cleanup handlers — releasing @@ -43,6 +45,15 @@ // the caller's own process group — the web server. Rejecting anything below 2 also // excludes init. // +// A group is never signalled unless the job leads it. +// pgid must equal the recorded pid, and must not equal this process's own group. This is +// the guard the pid < 2 check was mistaken for: pid was always a valid, live number, and +// the group it named was still the web server's. On 2026-08-07 stopping a genuinely +// running transcode_management.sh SIGTERMed then SIGKILLed the php-fpm group and took the +// WebGUI down; Docker was unaffected, so it presented as an OS crash. The failure needs a +// job that is still alive when Stop is pressed, which is why earlier stop tests — where +// the script had already exited and `ps` returned nothing — passed. +// // A job that is not running is a no-op success. // Both the missing-stat-file and status-not-running paths exit before any signal is // sent, so pressing stop twice cannot kill an unrelated process that has since been @@ -112,14 +123,29 @@ if ($pid < 2) { exit; } -// Kill the whole process group so the script and all its children die together. -// pgid is usually the same as the session leader PID from run_job.sh. -$pgid = (int)trim(shell_exec("ps -o pgid= -p $pid 2>/dev/null") ?: '0'); +// Kill the whole process group so the script and all its children die together — but only when +// the job genuinely owns that group. +// +// The old code took whatever `ps -o pgid=` returned and signalled it. The header claimed the pgid +// "is usually the same as the session leader PID from run_job.sh", and that was never true: neither +// `nohup` nor `&` starts a new process group, and PHP's exec() runs the command under a +// non-interactive `sh -c` where job control is off. The job therefore inherited the process group +// of the php-fpm worker that spawned it, and `kill -TERM -$pgid` signalled the entire web server +// group. Pressing Stop on a job that was actually still running took the WebGUI down with it; +// Docker lives in its own sessions and kept serving, which is what made it look like an OS crash +// rather than a plugin bug. run.php now prepends setsid so the job is its own leader, and this +// check is what holds even if it ever stops doing that. +$pgid = (int)trim(shell_exec("ps -o pgid= -p $pid 2>/dev/null") ?: '0'); +$ownPgid = function_exists('posix_getpgrp') ? (int)posix_getpgrp() : 0; -if ($pgid > 1) { +// A group kill is only ever safe when the recorded pid IS the group leader. Anything else means +// the group is shared with processes this endpoint knows nothing about. +$groupSafe = ($pgid > 1 && $pgid === $pid && $pgid !== $ownPgid); + +if ($groupSafe) { shell_exec("kill -TERM -$pgid 2>/dev/null"); } else { - // Fallback: kill the direct PID and its children + // Not our group — signal only the process and its direct children. shell_exec("pkill -TERM -P $pid 2>/dev/null"); shell_exec("kill -TERM $pid 2>/dev/null"); } @@ -133,7 +159,7 @@ for ($i = 0; $i < 6; $i++) { // Force-kill if still alive if (!$dead) { - if ($pgid > 1) shell_exec("kill -KILL -$pgid 2>/dev/null"); + if ($groupSafe) shell_exec("kill -KILL -$pgid 2>/dev/null"); shell_exec("pkill -KILL -P $pid 2>/dev/null"); shell_exec("kill -KILL $pid 2>/dev/null"); usleep(300000);