Survive a crash, and a reboot after one
kill, a closed window, a reboot: no SessionEnd arrives and the state file stays. Each case was tried rather than reasoned about, and one of the three was broken. A killed session was already handled -- the process is gone, so the file and its lock are removed within the 20 s liveness tick. An interrupted hook write left its temporary file behind forever; those are now swept once they are five minutes old, which is late enough that a hook part-way through writing one does not lose the update. The reboot case was the broken one. State files outlive a reboot and pids are handed out afresh, so "does /proc/<pid> exist" only answers "is some process wearing that number". Verified by giving an unrelated live process the pid of a dead session: the ghost sat in the panel as a session waiting for input, and would have stayed there forever, asking for an answer nobody could give. The pid is now pinned to the process start time from /proc/<pid>/stat, recorded when the state is written and compared when it is read. Files written before that field existed compare only on existence, as before, so a session open across the upgrade is not evicted. An abandoned flock needed nothing: the kernel drops it when the holder dies, so there is no deadlock to recover from.
This commit is contained in:
@@ -197,13 +197,33 @@ dump-layout`: рабочий каталог сессии сопоставляе
|
|||||||
Если zellij не используется, выключите в настройках: он стоит одного процесса
|
Если zellij не используется, выключите в настройках: он стоит одного процесса
|
||||||
раз в пару минут.
|
раз в пару минут.
|
||||||
|
|
||||||
|
## Аварийное завершение
|
||||||
|
|
||||||
|
`kill`, закрытое окно, перезагрузка — `SessionEnd` не приходит, и файл остаётся.
|
||||||
|
Разбор завалов проверен на каждом случае отдельно:
|
||||||
|
|
||||||
|
| Что осталось | Что с этим происходит |
|
||||||
|
|---|---|
|
||||||
|
| файл убитой сессии | процесса нет — файл и его `.lock` удаляются в пределах 20 с |
|
||||||
|
| файл из прошлой загрузки | pid сверяется по времени старта процесса, а не только по наличию |
|
||||||
|
| оборванная запись хука (`.tmp`) | удаляется, когда старше пяти минут |
|
||||||
|
| незакрытый `flock` | ядро снимает блокировку при смерти процесса — тупика не бывает |
|
||||||
|
|
||||||
|
Сверка по времени старта — не перестраховка. Файлы состояния переживают
|
||||||
|
перезагрузку, а pid после неё раздаются заново: проверка «есть ли `/proc/<pid>`»
|
||||||
|
отвечает лишь «какой-то процесс с таким номером есть». Без этой сверки сессия,
|
||||||
|
погибшая в аварии, висела бы в панели вечно, требуя ответа, которого некому дать.
|
||||||
|
Проверено подстановкой постороннего живого процесса на тот же pid.
|
||||||
|
|
||||||
|
Порог в пять минут для `.tmp` тоже осмысленный: хук может писать такой файл
|
||||||
|
прямо сейчас, и удаление свежего стоило бы потерянной записи.
|
||||||
|
|
||||||
|
Если хук вообще не смог опознать процесс claude, он пишет pid 0 — «неизвестно»,
|
||||||
|
что никогда не путается с «мёртв»; такие записи истекают по возрасту, через
|
||||||
|
36 часов.
|
||||||
|
|
||||||
## Известные шероховатости
|
## Известные шероховатости
|
||||||
|
|
||||||
- **Протухшие сессии.** Убитый терминал не присылает `SessionEnd`. Живость
|
|
||||||
перепроверяется каждые 20 с по `/proc/<pid>`, файл удаляется — убитая сессия
|
|
||||||
исчезает в пределах этого окна, а не висит вечно. Если хук вообще не смог
|
|
||||||
опознать процесс claude, он записывает pid 0 — «неизвестно», что никогда не
|
|
||||||
путается с «мёртв», — и такие записи истекают по возрасту, через 36 часов.
|
|
||||||
- **`Stop` не отличает «закончил» от «сдался».** Оба читаются как «ждёт ввода»,
|
- **`Stop` не отличает «закончил» от «сдался».** Оба читаются как «ждёт ввода»,
|
||||||
и решение для вас в обоих случаях одно и то же.
|
и решение для вас в обоих случаях одно и то же.
|
||||||
|
|
||||||
|
|||||||
@@ -169,8 +169,32 @@ def find_claude_pid():
|
|||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
|
||||||
def alive(pid):
|
def pid_start_time(pid):
|
||||||
return pid > 0 and os.path.exists("/proc/%d" % pid)
|
"""Field 22 of /proc/<pid>/stat: when the process started, in clock ticks.
|
||||||
|
|
||||||
|
Pins a pid to one particular process. Pids are reused, and state files
|
||||||
|
outlive reboots -- without this, a file left by a crashed session whose pid
|
||||||
|
is later handed to something unrelated reads as a live session forever.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
with open("/proc/%d/stat" % pid) as fh:
|
||||||
|
data = fh.read()
|
||||||
|
except OSError:
|
||||||
|
return 0
|
||||||
|
# Field 2 is the command name, parenthesised, and may itself contain spaces
|
||||||
|
# and a ')'. Everything after the last ')' is field 3 onwards.
|
||||||
|
tail = data[data.rfind(")") + 2:].split()
|
||||||
|
try:
|
||||||
|
return int(tail[19])
|
||||||
|
except (IndexError, ValueError):
|
||||||
|
return 0
|
||||||
|
|
||||||
|
|
||||||
|
def alive(pid, start=0):
|
||||||
|
if pid <= 0 or not os.path.exists("/proc/%d" % pid):
|
||||||
|
return False
|
||||||
|
# A file written before start times were recorded has nothing to compare.
|
||||||
|
return not start or pid_start_time(pid) == start
|
||||||
|
|
||||||
|
|
||||||
def sweep_dead(keep):
|
def sweep_dead(keep):
|
||||||
@@ -190,12 +214,13 @@ def sweep_dead(keep):
|
|||||||
path = os.path.join(STATE_DIR, name)
|
path = os.path.join(STATE_DIR, name)
|
||||||
try:
|
try:
|
||||||
with open(path, "r") as fh:
|
with open(path, "r") as fh:
|
||||||
pid = json.load(fh).get("pid", 0)
|
stale = json.load(fh)
|
||||||
|
pid = stale.get("pid", 0)
|
||||||
except (OSError, ValueError, AttributeError):
|
except (OSError, ValueError, AttributeError):
|
||||||
continue
|
continue
|
||||||
# pid 0 means the hook could not identify the process; there is nothing
|
# pid 0 means the hook could not identify the process; there is nothing
|
||||||
# to test for liveness, so leave it to the reader's age cutoff.
|
# to test for liveness, so leave it to the reader's age cutoff.
|
||||||
if pid and not alive(pid):
|
if pid and not alive(pid, stale.get("pid_start", 0)):
|
||||||
for victim in (path, path + ".lock"):
|
for victim in (path, path + ".lock"):
|
||||||
try:
|
try:
|
||||||
os.unlink(victim)
|
os.unlink(victim)
|
||||||
@@ -300,6 +325,7 @@ def apply_event(event, state, path, now):
|
|||||||
"event_ts": now,
|
"event_ts": now,
|
||||||
# 0 means "could not tell"; the reader must not take that for "dead".
|
# 0 means "could not tell"; the reader must not take that for "dead".
|
||||||
"pid": claude_pid,
|
"pid": claude_pid,
|
||||||
|
"pid_start": pid_start_time(claude_pid) if claude_pid else 0,
|
||||||
"event": event.get("hook_event_name", ""),
|
"event": event.get("hook_event_name", ""),
|
||||||
"notification_type": event.get("notification_type", ""),
|
"notification_type": event.get("notification_type", ""),
|
||||||
"message": message,
|
"message": message,
|
||||||
|
|||||||
+59
-6
@@ -27,6 +27,12 @@ const LIVENESS_INTERVAL = 20; // seconds
|
|||||||
// morning, which is exactly the case this indicator exists for.
|
// morning, which is exactly the case this indicator exists for.
|
||||||
const UNKNOWN_PID_MAX_AGE = 36 * 3600; // seconds
|
const UNKNOWN_PID_MAX_AGE = 36 * 3600; // seconds
|
||||||
|
|
||||||
|
// A hook killed between writing its temporary file and renaming it leaves the
|
||||||
|
// temporary behind. Old ones are swept; recent ones are left alone, because a
|
||||||
|
// hook may be part-way through writing one right now and deleting it would
|
||||||
|
// lose that update.
|
||||||
|
const TMP_MAX_AGE = 300; // seconds
|
||||||
|
|
||||||
export function stateRank(state) {
|
export function stateRank(state) {
|
||||||
const i = STATES.indexOf(state);
|
const i = STATES.indexOf(state);
|
||||||
return i < 0 ? STATES.length : i;
|
return i < 0 ? STATES.length : i;
|
||||||
@@ -128,7 +134,7 @@ export const SessionStore = GObject.registerClass({
|
|||||||
let enumerator;
|
let enumerator;
|
||||||
try {
|
try {
|
||||||
enumerator = await this._dir.enumerate_children_async(
|
enumerator = await this._dir.enumerate_children_async(
|
||||||
'standard::name', Gio.FileQueryInfoFlags.NONE,
|
'standard::name,time::modified', Gio.FileQueryInfoFlags.NONE,
|
||||||
GLib.PRIORITY_DEFAULT, cancellable);
|
GLib.PRIORITY_DEFAULT, cancellable);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
// No directory yet means no sessions have ever run; not an error.
|
// No directory yet means no sessions have ever run; not an error.
|
||||||
@@ -145,10 +151,12 @@ export const SessionStore = GObject.registerClass({
|
|||||||
break;
|
break;
|
||||||
for (const info of batch) {
|
for (const info of batch) {
|
||||||
const name = info.get_name();
|
const name = info.get_name();
|
||||||
// ".tmp" files are half-written state; "debug" is the hook's
|
// Only ".json" is state. ".lock" belongs to the hook, "debug"
|
||||||
// opt-in event log and is not a session.
|
// is its opt-in event log, and ".tmp" is an interrupted write.
|
||||||
if (name.endsWith('.json'))
|
if (name.endsWith('.json'))
|
||||||
names.push(name);
|
names.push(name);
|
||||||
|
else if (name.endsWith('.tmp'))
|
||||||
|
this._sweepTemp(info, name);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -161,6 +169,23 @@ export const SessionStore = GObject.registerClass({
|
|||||||
return sessions;
|
return sessions;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Delete an abandoned temporary file, once it is old enough to be sure. */
|
||||||
|
_sweepTemp(info, name) {
|
||||||
|
const modified = info.get_modification_date_time?.();
|
||||||
|
if (!modified)
|
||||||
|
return;
|
||||||
|
const age = GLib.DateTime.new_now_local().difference(modified) / 1e6;
|
||||||
|
if (age < TMP_MAX_AGE)
|
||||||
|
return;
|
||||||
|
this._dir.get_child(name).delete_async(GLib.PRIORITY_LOW, null, (obj, res) => {
|
||||||
|
try {
|
||||||
|
obj.delete_finish(res);
|
||||||
|
} catch (e) {
|
||||||
|
// Gone already, or not ours to remove.
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
async _readOne(name, cancellable) {
|
async _readOne(name, cancellable) {
|
||||||
const file = this._dir.get_child(name);
|
const file = this._dir.get_child(name);
|
||||||
let raw;
|
let raw;
|
||||||
@@ -176,12 +201,13 @@ export const SessionStore = GObject.registerClass({
|
|||||||
return null;
|
return null;
|
||||||
|
|
||||||
const pid = Number(raw.pid) || 0;
|
const pid = Number(raw.pid) || 0;
|
||||||
|
const pidStart = Number(raw.pid_start) || 0;
|
||||||
const eventTs = Number(raw.event_ts) || 0;
|
const eventTs = Number(raw.event_ts) || 0;
|
||||||
const age = GLib.get_real_time() / 1e6 - eventTs;
|
const age = GLib.get_real_time() / 1e6 - eventTs;
|
||||||
// pid 0 is "the hook could not tell", not "dead": treating it as dead
|
// pid 0 is "the hook could not tell", not "dead": treating it as dead
|
||||||
// would hide a perfectly live session, so those fall back to an age
|
// would hide a perfectly live session, so those fall back to an age
|
||||||
// cutoff instead.
|
// cutoff instead.
|
||||||
const gone = pid > 0 ? !isAlive(pid) : age > UNKNOWN_PID_MAX_AGE;
|
const gone = pid > 0 ? !isAlive(pid, pidStart) : age > UNKNOWN_PID_MAX_AGE;
|
||||||
if (gone) {
|
if (gone) {
|
||||||
// The terminal was killed without a SessionEnd hook. Removing the
|
// The terminal was killed without a SessionEnd hook. Removing the
|
||||||
// file here (rather than only hiding it) keeps the directory from
|
// file here (rather than only hiding it) keeps the directory from
|
||||||
@@ -234,6 +260,33 @@ export const SessionStore = GObject.registerClass({
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
function isAlive(pid) {
|
/** Is this pid still the process the hook recorded?
|
||||||
return pid > 0 && GLib.file_test(`/proc/${pid}`, GLib.FileTest.EXISTS);
|
*
|
||||||
|
* Existence alone is not enough. State files outlive reboots, and a pid from a
|
||||||
|
* previous boot is very likely to belong to something else now -- a session
|
||||||
|
* that died in a crash would otherwise sit in the panel forever, waiting for
|
||||||
|
* an answer nobody can give. The start time pins the pid to one process.
|
||||||
|
*/
|
||||||
|
function isAlive(pid, startTime) {
|
||||||
|
if (pid <= 0 || !GLib.file_test(`/proc/${pid}`, GLib.FileTest.EXISTS))
|
||||||
|
return false;
|
||||||
|
// Files written before start times were recorded have nothing to compare.
|
||||||
|
if (!startTime)
|
||||||
|
return true;
|
||||||
|
return readStartTime(pid) === startTime;
|
||||||
|
}
|
||||||
|
|
||||||
|
function readStartTime(pid) {
|
||||||
|
try {
|
||||||
|
const [ok, bytes] = GLib.file_get_contents(`/proc/${pid}/stat`);
|
||||||
|
if (!ok)
|
||||||
|
return 0;
|
||||||
|
const data = new TextDecoder().decode(bytes);
|
||||||
|
// The command name is parenthesised and may contain spaces and ')',
|
||||||
|
// so fields are counted from after the last one.
|
||||||
|
const tail = data.slice(data.lastIndexOf(')') + 2).split(' ');
|
||||||
|
return Number(tail[19]) || 0;
|
||||||
|
} catch (e) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,6 +36,11 @@ ev() { # event [extra json]
|
|||||||
emit "$(ev SessionStart '"source":"startup"')"
|
emit "$(ev SessionStart '"source":"startup"')"
|
||||||
check "SessionStart -> waiting" "waiting" "$(field state)"
|
check "SessionStart -> waiting" "waiting" "$(field state)"
|
||||||
|
|
||||||
|
# Pins the recorded pid to one process, so a state file that outlives a reboot
|
||||||
|
# cannot be revived by whatever inherits that pid number next.
|
||||||
|
start=$(field pid_start)
|
||||||
|
check "process start time recorded" "yes" "$([ -n "$start" ] && [ "$start" != "0" ] && echo yes || echo no)"
|
||||||
|
|
||||||
emit "$(ev UserPromptSubmit '"prompt":"hi"')"
|
emit "$(ev UserPromptSubmit '"prompt":"hi"')"
|
||||||
check "UserPromptSubmit -> busy" "busy" "$(field state)"
|
check "UserPromptSubmit -> busy" "busy" "$(field state)"
|
||||||
|
|
||||||
|
|||||||
+11
-3
@@ -27,11 +27,12 @@ function check(name, condition, detail = '') {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const now = GLib.get_real_time() / 1e6;
|
const now = GLib.get_real_time() / 1e6;
|
||||||
function write(id, state, cwd, agoSeconds, pid) {
|
function write(id, state, cwd, agoSeconds, pid, pidStart = 0) {
|
||||||
const payload = {
|
const payload = {
|
||||||
session_id: id, state, cwd, since: now - agoSeconds,
|
session_id: id, state, cwd, since: now - agoSeconds,
|
||||||
event_ts: now, pid, event: 'test', notification_type: '',
|
event_ts: now, pid, pid_start: pidStart, event: 'test',
|
||||||
message: '', zellij_session: 'ztest', zellij_pane: '1', transcript: '',
|
notification_type: '', message: '', zellij_session: 'ztest',
|
||||||
|
zellij_pane: '1', transcript: '',
|
||||||
};
|
};
|
||||||
GLib.file_set_contents(
|
GLib.file_set_contents(
|
||||||
GLib.build_filenamev([STATE, `${id}.json`]), JSON.stringify(payload));
|
GLib.build_filenamev([STATE, `${id}.json`]), JSON.stringify(payload));
|
||||||
@@ -50,6 +51,9 @@ write('s-dead', 'waiting', '/home/u/proj-dead', 5, deadPid);
|
|||||||
// Written by the older hook, which had a fourth state. Files like this
|
// Written by the older hook, which had a fourth state. Files like this
|
||||||
// survive an upgrade in a session that was already open.
|
// survive an upgrade in a session that was already open.
|
||||||
write('s-legacy', 'idle', '/home/u/proj-legacy', 5, livePid);
|
write('s-legacy', 'idle', '/home/u/proj-legacy', 5, livePid);
|
||||||
|
// Survived a reboot: the pid exists again, but belongs to something else now.
|
||||||
|
// Without an identity check this sits in the panel forever as a live session.
|
||||||
|
write('s-ghost', 'waiting', '/home/u/proj-ghost', 99999, livePid, 1);
|
||||||
GLib.file_set_contents(GLib.build_filenamev([STATE, 'notes.txt']), 'ignored');
|
GLib.file_set_contents(GLib.build_filenamev([STATE, 'notes.txt']), 'ignored');
|
||||||
GLib.file_set_contents(GLib.build_filenamev([STATE, 'half.json']), '{"broken');
|
GLib.file_set_contents(GLib.build_filenamev([STATE, 'half.json']), '{"broken');
|
||||||
|
|
||||||
@@ -67,6 +71,10 @@ store.connect('changed', () => {
|
|||||||
stateRank('waiting') < stateRank('busy'));
|
stateRank('waiting') < stateRank('busy'));
|
||||||
|
|
||||||
check('dead session dropped', !s.some(x => x.sessionId === 's-dead'));
|
check('dead session dropped', !s.some(x => x.sessionId === 's-dead'));
|
||||||
|
check('reused pid from a previous boot dropped',
|
||||||
|
!s.some(x => x.sessionId === 's-ghost'));
|
||||||
|
check('and its file removed',
|
||||||
|
!GLib.file_test(GLib.build_filenamev([STATE, 's-ghost.json']), GLib.FileTest.EXISTS));
|
||||||
check('truncated file skipped, others survive', s.length === 5,
|
check('truncated file skipped, others survive', s.length === 5,
|
||||||
`got ${s.length}: ${s.map(x => x.sessionId).join(',')}`);
|
`got ${s.length}: ${s.map(x => x.sessionId).join(',')}`);
|
||||||
check('non-json ignored', !s.some(x => x.sessionId.includes('notes')));
|
check('non-json ignored', !s.some(x => x.sessionId.includes('notes')));
|
||||||
|
|||||||
Reference in New Issue
Block a user