From 8e06b30cd8b8b767f2e857be9360068c9585cee2 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Sat, 1 Aug 2026 02:00:35 +0000 Subject: [PATCH] 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 --- apps/desktop/electron/main.ts | 15 ++++++- apps/desktop/electron/update-marker.test.ts | 49 +++++++++++++++++++++ apps/desktop/electron/update-marker.ts | 44 ++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index eb86d600a1964..65395c499db77 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -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: diff --git a/apps/desktop/electron/update-marker.test.ts b/apps/desktop/electron/update-marker.test.ts index 3da49396cc8d8..0fb1142b4cbc1 100644 --- a/apps/desktop/electron/update-marker.test.ts +++ b/apps/desktop/electron/update-marker.test.ts @@ -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/) +}) diff --git a/apps/desktop/electron/update-marker.ts b/apps/desktop/electron/update-marker.ts index 543fce0451d08..ee50b52e21147 100644 --- a/apps/desktop/electron/update-marker.ts +++ b/apps/desktop/electron/update-marker.ts @@ -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.` + } +}