diff --git a/Plugin/unraid/api/stop.php b/Plugin/unraid/api/stop.php index a9f8f2a..1ce6797 100644 --- a/Plugin/unraid/api/stop.php +++ b/Plugin/unraid/api/stop.php @@ -55,9 +55,24 @@ // 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 -// assigned the recorded pid. +// Three paths exit before any signal is sent: no stat file, status not 'running', and a +// recorded pid with no /proc entry. The third is the one that matters, because the other +// two only cover a job that got to write its own ending. run_job.sh updates the stat file +// on exit, but SIGKILL, an OOM kill and a power loss all skip that — leaving 'running' +// and a pid the kernel is free to hand to something else. Signalling on the strength of +// that file alone is signalling a stranger. run.php and status.php already refuse to +// believe a 'running' stat without the /proc check; this endpoint is the one that sends +// SIGKILL, so it is the last place that should have been taking the file's word for it. +// +// The stale entry is corrected to 'error' on the way out rather than left alone: the job +// did not stop, it died, and a status of 'stopped' would claim this endpoint did +// something it did not do. +// +// What this does not prove is that a live /proc/ is still *this* job's process — +// a pid reissued to something unrelated between the stat write and the click looks +// identical. Closing that needs the process start-time out of /proc//stat compared +// against the recorded start. Not done here, and deliberately: it is real machinery, and +// the window it closes is far narrower than the one above. // // The pid is only cleared from the stat file if the process is confirmed gone. // D-state processes survive SIGKILL. Clearing the pid there would lose the only handle @@ -85,6 +100,7 @@ // RESPONSE // {"ok":true,"killed":true,"locks":["…"],"error":null} // {"ok":true,"msg":"Not running"} +// {"ok":true,"msg":"Not running — stale pid cleared"} stat said running, process was gone // {"ok":false,"killed":false,"locks":[…], // "error":"Process still alive after SIGKILL (D-state) — lock may persist"} // {"ok":false,"error":"Invalid id"|"No stat file — script may not be running" @@ -123,6 +139,19 @@ if ($pid < 2) { exit; } +// The stat file says 'running'; the process table is what decides whether that is still true. +// A job killed hard never got to correct its own file, so 'running' can outlive the process by +// days and the pid can since have been reissued. Correct the record and stop — nothing here is +// worth signalling. +if (!file_exists("/proc/$pid")) { + $stat['status'] = 'error'; + $stat['end'] = $stat['end'] ?? time(); + unset($stat['pid']); + file_put_contents($statFile, json_encode($stat)); + echo json_encode(['ok' => true, 'msg' => 'Not running — stale pid cleared']); + exit; +} + // Kill the whole process group so the script and all its children die together — but only when // the job genuinely owns that group. // diff --git a/Plugin/unraid/event/disks_mounted/array_start_jobs b/Plugin/unraid/event/disks_mounted/array_start_jobs index cc0976d..34d33b2 100755 --- a/Plugin/unraid/event/disks_mounted/array_start_jobs +++ b/Plugin/unraid/event/disks_mounted/array_start_jobs @@ -13,6 +13,8 @@ foreach (\$s as \$id => \$e) { : SCRIPTS_DIR . '/' . \$id; if (!file_exists(\$script)) continue; \$flags = !empty(\$e['log_enabled']) ? ' --log' : ''; - exec('nohup bash ' . escapeshellarg(\$runner) . ' ' . escapeshellarg(\$id) . ' ' . escapeshellarg(\$script) . \$flags . ' > /dev/null 2>&1 &'); + // setsid for the same reason as api/run.php: the job must lead its own process group or + // api/stop.php cannot signal the tree, only the parent and its direct children. + exec('setsid nohup bash ' . escapeshellarg(\$runner) . ' ' . escapeshellarg(\$id) . ' ' . escapeshellarg(\$script) . \$flags . ' > /dev/null 2>&1 &'); } " 2>/dev/null