fix(desktop): refuse a second update hand-off while one is already live
writeUpdateMarker unconditionally overwrites HERMES_HOME/.hermes-update-in-progress before every hand-off. If the user retries "Update" while a prior updater is still alive and parked (e.g. waiting for the desktop to exit), the retry's pre-write clobbers the still-running updater's claim, so the older updater is no longer recorded as the owner even though it's actively mutating the checkout. A second updater can then run concurrently over the same tree. Add updateHandoffConflict() to check for a live foreign marker owner before spawning a new updater, and refuse the hand-off (surfacing an "update already running" message) instead of overwriting the marker. Ref: #75778
This commit is contained in:
parent
5eeafc8d25
commit
8e06b30cd8
|
|
@ -189,7 +189,7 @@ import { createStreamThrottle } from './stream-throttle'
|
|||
import { nativeOverlayWidth as computeNativeOverlayWidth, macTitleBarOverlayHeight } from './titlebar-overlay-width'
|
||||
import { resolveBehindCount, shouldCountCommits } from './update-count'
|
||||
import { waitForUpdateClearance } from './update-gate'
|
||||
import { readLiveUpdateMarker, writeUpdateMarker } from './update-marker'
|
||||
import { readLiveUpdateMarker, updateHandoffConflict, writeUpdateMarker } from './update-marker'
|
||||
import { runRebuildWithRetry } from './update-rebuild'
|
||||
import {
|
||||
buildRelaunchScript,
|
||||
|
|
@ -2898,6 +2898,19 @@ async function applyUpdates(opts = {}) {
|
|||
return { ok: true, manual: true, command, hermesRoot: updateRoot }
|
||||
}
|
||||
|
||||
const handoffConflict = updateHandoffConflict(HERMES_HOME)
|
||||
|
||||
if (handoffConflict) {
|
||||
// A different updater already owns the marker — most often a previous
|
||||
// "Update" click whose updater is still alive and parked mid-run.
|
||||
// Spawning another here would overwrite its claim and let two updaters
|
||||
// mutate the checkout at once (#75778); refuse instead.
|
||||
rememberLog(`[updates] refusing hand-off: ${handoffConflict.message}`)
|
||||
emitUpdateProgress({ stage: 'error', message: handoffConflict.message, percent: null })
|
||||
|
||||
return { ok: false, error: 'update-already-running', message: handoffConflict.message }
|
||||
}
|
||||
|
||||
emitUpdateProgress({
|
||||
stage: 'restart',
|
||||
message:
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import {
|
|||
markerPath,
|
||||
readLiveUpdateMarker,
|
||||
UPDATE_MARKER_MAX_AGE_MS,
|
||||
updateHandoffConflict,
|
||||
writeUpdateMarker
|
||||
} from './update-marker'
|
||||
|
||||
|
|
@ -128,3 +129,51 @@ test('writeUpdateMarker + dead pid => self-heals on read', () => {
|
|||
assert.equal(res, null, 'a dead-pid marker from writeUpdateMarker self-heals')
|
||||
assert.ok(!fs.existsSync(markerPath(home)), 'marker file is pruned')
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// updateHandoffConflict (#75778)
|
||||
//
|
||||
// A retried "Update" click must not spawn a second updater over a still-live
|
||||
// one — writeUpdateMarker unconditionally overwrites the marker, so an
|
||||
// unchecked hand-off clobbers the original updater's claim while it is still
|
||||
// alive and mutating the checkout.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('no marker => hand-off is not blocked', () => {
|
||||
const home = tmpHome('conflict-none')
|
||||
assert.equal(updateHandoffConflict(home, { kill: ALIVE }), null)
|
||||
})
|
||||
|
||||
test('a different live updater already owns the marker => hand-off is blocked', () => {
|
||||
const home = tmpHome('conflict-live')
|
||||
const now = 1_000_000_000_000
|
||||
writeMarker(home, 1010, Math.floor(now / 1000) - 6) // 6s old
|
||||
const conflict = updateHandoffConflict(home, { kill: ALIVE, now: () => now })
|
||||
assert.ok(conflict, 'a live foreign updater must block a new hand-off')
|
||||
assert.equal(conflict.pid, 1010)
|
||||
assert.match(conflict.message, /already running/)
|
||||
assert.match(conflict.message, /PID 1010/)
|
||||
assert.match(conflict.message, /6s/)
|
||||
})
|
||||
|
||||
test('a dead-pid marker does not block a hand-off (self-heals)', () => {
|
||||
const home = tmpHome('conflict-dead')
|
||||
writeMarker(home, 999999, Math.floor(Date.now() / 1000))
|
||||
assert.equal(updateHandoffConflict(home, { kill: DEAD }), null)
|
||||
})
|
||||
|
||||
test('an expired marker does not block a hand-off (self-heals)', () => {
|
||||
const home = tmpHome('conflict-expired')
|
||||
const now = 1_000_000_000_000
|
||||
writeMarker(home, 1010, Math.floor((now - UPDATE_MARKER_MAX_AGE_MS - 60_000) / 1000))
|
||||
assert.equal(updateHandoffConflict(home, { kill: ALIVE, now: () => now }), null)
|
||||
})
|
||||
|
||||
test('minutes-scale elapsed time is formatted as "Nm Ss"', () => {
|
||||
const home = tmpHome('conflict-minutes')
|
||||
const now = 1_000_000_000_000
|
||||
writeMarker(home, 1010, Math.floor(now / 1000) - 125) // 2m 5s old
|
||||
const conflict = updateHandoffConflict(home, { kill: ALIVE, now: () => now })
|
||||
assert.ok(conflict)
|
||||
assert.match(conflict.message, /2m 5s/)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -136,3 +136,47 @@ export function writeUpdateMarker(hermesHome, pid, { now = Date.now } = {}) {
|
|||
// updater will write its own when it reaches run_update.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a NEW updater hand-off must be refused because a different,
|
||||
* already-alive updater currently owns the marker (#75778).
|
||||
*
|
||||
* `writeUpdateMarker` unconditionally overwrites the marker file. Called
|
||||
* before every hand-off with no conflict check, a user who clicks "Update"
|
||||
* again while a prior updater is still parked mid-run (e.g. "waiting for
|
||||
* Hermes to exit…") clobbers that still-running updater's claim: the
|
||||
* retry's pre-write now names the NEW child, so the OLD process — alive
|
||||
* and mutating the checkout — is no longer recorded as the owner. A second
|
||||
* live updater can then run over the same tree unrecorded, the exact
|
||||
* two-updaters-at-once hazard `UpdateMarkerGuard` in the Rust updater
|
||||
* exists to prevent (apps/bootstrap-installer/src-tauri/src/update.rs).
|
||||
*
|
||||
* Returns the live foreign owner (with a ready-to-show message) when the
|
||||
* hand-off must be refused, or `null` when it's safe to spawn — no marker,
|
||||
* or the existing one is stale/dead and self-heals via
|
||||
* `readLiveUpdateMarker`.
|
||||
*/
|
||||
export function updateHandoffConflict(
|
||||
hermesHome,
|
||||
opts: {
|
||||
now?: () => number
|
||||
maxAgeMs?: number
|
||||
kill?: typeof process.kill
|
||||
} = {}
|
||||
) {
|
||||
const owner = readLiveUpdateMarker(hermesHome, opts)
|
||||
|
||||
if (!owner) {
|
||||
return null
|
||||
}
|
||||
|
||||
const mins = Math.floor(owner.ageMs / 60_000)
|
||||
const secs = Math.floor((owner.ageMs % 60_000) / 1000)
|
||||
const elapsed = mins > 0 ? `${mins}m ${secs}s` : `${secs}s`
|
||||
|
||||
return {
|
||||
pid: owner.pid,
|
||||
ageMs: owner.ageMs,
|
||||
message: `An update is already running (PID ${owner.pid}, started ${elapsed} ago). Wait for it to finish, then try again.`
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue