mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 00:03:47 +00:00
fix(repro): stop the script doing the exact thing it exists to warn about
Both findings are right, and the first is pointed: this script demonstrates that signalling a pid you have not re-verified can kill someone else's work, and its own teardown did that twice. The staged daemon is killed on purpose in phase 1. Once Node reaps that child its pid is free for reuse, and teardown signalled the remembered number anyway; it now signals only while the child object still reports no exit. The markers were half-verified. Each proves its own identity by tag before being signalled, but its session leader was killed on a number remembered from staging a minute earlier — and the leader is the one pid here that can be recycled while its child lives on under a new parent. The leader is now re-read from the live marker and signalled only when the marker still claims it. Second finding: isRealUserDaemon matched a hardcoded macOS userData path, so on Linux a real daemon could never be recognised — the assertion that this run harmed nothing was inert on the platform where nobody would notice. Now matched per platform, with a loose fallback rather than a silent false. Repro re-run end to end: all three phases pass, pre-existing daemons still running, real userData daemon untouched.
This commit is contained in:
@@ -50,6 +50,7 @@ import { checkLauncherHoldsOccupiedDaemon } from './daemon-replacement-launcher-
|
||||
import {
|
||||
findTaggedPid,
|
||||
isMarkerAlive,
|
||||
verifiedSessionLeaderPid,
|
||||
isProcessAlive,
|
||||
processArgs,
|
||||
processState,
|
||||
@@ -429,7 +430,14 @@ function teardown(staged) {
|
||||
if (!staged) {
|
||||
return
|
||||
}
|
||||
const daemonPid = staged.daemon?.child.pid
|
||||
// Why the exit check: phase 1 kills this daemon on purpose, and once Node has reaped the
|
||||
// child its pid is free for the OS to reuse. Signalling the remembered number after that is
|
||||
// signalling a stranger.
|
||||
const daemonChild = staged.daemon?.child
|
||||
const daemonPid =
|
||||
daemonChild && daemonChild.exitCode === null && daemonChild.signalCode === null
|
||||
? daemonChild.pid
|
||||
: undefined
|
||||
if (daemonPid) {
|
||||
for (const signal of staged.stopped ? ['SIGCONT', 'SIGKILL'] : ['SIGKILL']) {
|
||||
try {
|
||||
@@ -451,7 +459,13 @@ function teardown(staged) {
|
||||
if (!isMarkerAlive(marker)) {
|
||||
continue
|
||||
}
|
||||
for (const pid of [marker.pid, marker.sessionPid]) {
|
||||
// The leader is re-read from the live marker rather than remembered: the marker proves its
|
||||
// own identity by tag, but nothing proved the leader's, and it is the one pid here that
|
||||
// could have been recycled while its child stayed alive under a new parent.
|
||||
for (const pid of [marker.pid, verifiedSessionLeaderPid(marker)]) {
|
||||
if (!pid) {
|
||||
continue
|
||||
}
|
||||
try {
|
||||
process.kill(pid, 'SIGKILL')
|
||||
} catch {
|
||||
|
||||
@@ -6,7 +6,15 @@
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process'
|
||||
|
||||
const REAL_USER_DAEMON_MARKER = 'Library/Application Support/orca/daemon'
|
||||
// Electron's userData path differs per platform, and hardcoding the macOS one meant a real
|
||||
// daemon could never be recognised on Linux — so the guard that this run harmed nothing was
|
||||
// inert on exactly the platform where it would go unnoticed.
|
||||
const REAL_USER_DAEMON_MARKERS = {
|
||||
darwin: ['Library/Application Support/orca/daemon'],
|
||||
linux: ['.config/orca/daemon'],
|
||||
win32: ['AppData/Roaming/orca/daemon', 'AppData\\Roaming\\orca\\daemon']
|
||||
}
|
||||
const REAL_USER_DAEMON_MARKER_LIST = REAL_USER_DAEMON_MARKERS[process.platform] ?? ['orca/daemon']
|
||||
|
||||
export function processArgs(pid) {
|
||||
try {
|
||||
@@ -66,6 +74,31 @@ export function isMarkerAlive(marker) {
|
||||
return processArgs(marker.pid)?.includes(marker.tag) === true
|
||||
}
|
||||
|
||||
/**
|
||||
* The session leader of a still-live marker, read now rather than remembered.
|
||||
*
|
||||
* Why not trust the pid captured at staging: teardown runs a minute later, and phase 1 has
|
||||
* deliberately killed things in between. A remembered leader pid may by then belong to whatever
|
||||
* the OS recycled it onto, and SIGKILLing that is precisely the mistake this script exists to
|
||||
* demonstrate. Returns null unless the live marker still claims this leader.
|
||||
*/
|
||||
export function verifiedSessionLeaderPid(marker) {
|
||||
if (!isMarkerAlive(marker)) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
const ppid = Number(
|
||||
execFileSync('ps', ['-p', String(marker.pid), '-o', 'ppid='], {
|
||||
encoding: 'utf8',
|
||||
timeout: 5_000
|
||||
}).trim()
|
||||
)
|
||||
return Number.isInteger(ppid) && ppid === marker.sessionPid ? ppid : null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
// Pre-existing daemons (the user's real one above all) must be untouched by this run.
|
||||
export function snapshotForeignDaemons() {
|
||||
const daemons = []
|
||||
@@ -77,7 +110,10 @@ export function snapshotForeignDaemons() {
|
||||
}
|
||||
const pid = Number(line.trim().split(/\s+/, 1)[0])
|
||||
if (Number.isInteger(pid) && pid > 0) {
|
||||
daemons.push({ pid, isRealUserDaemon: line.includes(REAL_USER_DAEMON_MARKER) })
|
||||
daemons.push({
|
||||
pid,
|
||||
isRealUserDaemon: REAL_USER_DAEMON_MARKER_LIST.some((marker) => line.includes(marker))
|
||||
})
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user