Stop signalling on the strength of a stat file alone
A job killed hard never corrects its own record, so 'running' outlives the process and the pid can since belong to something else; array-start jobs get setsid too, so Stop can reach past their direct children.
This commit is contained in:
@@ -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/<pid> 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/<pid>/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.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user