diff --git a/report.md b/report.md new file mode 100644 index 00000000000000..628fd1f79ee1ad --- /dev/null +++ b/report.md @@ -0,0 +1,203 @@ +# Issue Analysis Report: microsoft/vscode#62476 + +## Scope + +This report analyzes `https://github.com/microsoft/vscode/issues/62476` end-to-end, including all issue comments and linked/duplicate issues (including duplicate ones), and proposes actionable repro scenarios plus an implementation plan. + +Repositories considered: + +- `vscode` (product updater orchestration and startup checks) +- `inno-updater` (process wait/kill behavior during apply) +- `vscode-update-server` (update metadata/feed behavior and cache) + +## Primary Symptom + +Windows users intermittently see an Inno Setup dialog: + +`Setup has detected that setup is currently running` + +The prompt appears unexpectedly, often while VS Code itself is running and near update activity (`Restart to Update` flow), and can recur repeatedly. + +## Evidence Summary (Issue + Comments) + +Issue `#62476` contains long-running reports across multiple years. The high-signal evidence in comments clusters into the following patterns: + +1. Update race during background apply and relaunch/reopen. + +- Maintainer repro hypothesis (Nov 2018) explicitly describes a second instance launching setup while a prior setup/apply is still in progress. +- Multiple users report correlation with opening another VS Code window while update is pending. + +1. Recovery path collisions after interrupted update. + +- Reports describe closing/reopening VS Code mid-update, then seeing setup-running dialogs on next launch. +- This aligns with stale pending-update recovery paths (`updating_version`) attempting to apply while setup may still be active. + +1. Stuck helper/setup process behavior. + +- Reports include persistent `CodeSetup*.exe` or update helper process remaining active and repeated dialogs until process kill/reboot. +- Later comments suggest killing update-related PowerShell process can unblock update completion in some cases. + +1. UX confusion. + +- Users often cannot identify the dialog as VS Code related. +- Repeated modal behavior is considered highly disruptive. + +## Linked And Duplicate Issues (Reviewed) + +The following linked/duplicate issues were reviewed, including their comments: + +1. `#261687` (`"Setup" pop-up when opening up VSCode`) - closed as duplicate of `#62476` + +- Repro pattern: popup on open after update cycle. +- Mentions install stuck at `Installing...` in user narrative. + +1. `#262945` (`Stuck on "Installing Update..."`) - closed as duplicate + +- Explicitly references `#62476` behavior after reopen. +- Provides workaround: terminate lingering PowerShell process. + +1. `#147287` (`Opening VS Code gave following error.`) - closed as duplicate + +- Triggered after forcing close of unresponsive VS Code, then reopen. + +1. `#193277` (`Lost Issue #62476`) - closed as duplicate/meta tracking + +- Does not add technical root-cause details, but confirms issue remained active and assigned. + +## Technical Mapping To Code + +### 1) VS Code product updater (`vscode`) + +Main orchestration points: + +- `src/vs/platform/update/electron-main/updateService.win32.ts` + - `postInitialize()` reads `updating_version` and can re-enter apply via `_applySpecificUpdate(...)`. + - `doApplyUpdate()` spawns installer silently and polls ready mutex/progress. + - `_applySpecificUpdate(...)` sets update state and can call `doApplyUpdate()`. + +Startup gate: + +- `src/vs/code/electron-main/main.ts` + - `checkInnoSetupMutex(...)` checks update mutex during startup and can block launch. + +Installer mutex semantics: + +- `build/win32/code.iss` + - `GetSetupMutex(...)` defines setup mutex behavior, including base `...setup` and background `...-updating` mutex. + +### 2) Inno updater (`inno-updater`) + +- `inno-updater/src/process.rs` + - `wait_or_kill(...)` waits and then attempts process termination for running executable conflicts. +- `inno-updater/src/main.rs` + - Integrates `wait_or_kill(...)` into update flow. + +This area explains some stuck-process persistence but does not appear to be the primary source of duplicate setup launch attempts. + +### 3) Update server (`vscode-update-server`) + +- `server/builds.ts` and `server/cache.ts` govern build selection, rollout, and cache freshness. + +Server behavior influences update cadence and metadata selection, but the popup symptom is primarily client-side installer coordination. + +## Repro Scenarios + +### Scenario 1: Multi-window background update race + +Preconditions: + +- Windows user setup build with background updates enabled. + +Steps: + +1. Start VS Code and wait until update is downloaded and background apply starts. +2. Before apply fully settles, open another VS Code window (or relaunch quickly). +3. Second instance enters apply path while installer from first path is still active. + +Expected result: + +- Second setup invocation collides with active setup mutex and surfaces `Setup has detected that setup is currently running`. + +### Scenario 2: Restart/reopen during in-progress update + +Preconditions: + +- Update started; recovery markers/state (e.g. `updating_version`) present. + +Steps: + +1. Let update apply begin. +2. Close and reopen VS Code before apply finalizes. +3. Startup recovery path attempts specific apply while installer may still be active. + +Expected result: + +- Recovery re-entry collides with active setup/update mutex and shows setup-running popup. + +### Scenario 3: Hung helper process loop + +Preconditions: + +- Update reaches `Installing Update...` and helper process does not exit cleanly. + +Steps: + +1. Trigger update and wait until install appears stalled. +2. Reopen VS Code or retrigger update check/apply. +3. Existing hung process remains active while new apply attempt is made. + +Expected result: + +- Repeated setup-running popup until lingering process exits or is terminated. + +## Implementation Plan + +### Item 1: Prevent duplicate setup launch in apply path + +- In `doApplyUpdate()`, check installer activity mutexes before spawning setup. +- If installer is already active, do not spawn a second setup process. +- Continue with state handling/polling logic needed to detect readiness or failure. + +### Item 2: Align mutex semantics across startup and apply + +- Use consistent mutex interpretation across: + - startup gate (`checkInnoSetupMutex`) + - update apply guard (`doApplyUpdate`) +- Consider both base setup mutex (`...setup`) and update mutex (`...-updating`) where appropriate. + +### Item 3: Harden stale recovery paths + +- Ensure `postInitialize()` / `_applySpecificUpdate()` cannot retrigger setup launch while installer is already active. +- Recovery should converge safely to: + - waiting for active installer completion, or + - explicit failure state when installer activity disappears without becoming ready. + +### Item 5: Improve diagnostics and telemetry breadcrumbs + +Add explicit breadcrumbs for: + +- duplicate launch prevented due to active installer mutex +- startup/update recovery seeing active installer mutex +- polling timeout / resume failure transitions + +Diagnostics should be low-cardinality and safe for telemetry use. + +## Risks And Open Questions + +1. Mutex naming/coverage must match installer semantics exactly (`code.iss` and app checks) to avoid false negatives. +2. Polling behavior must avoid indefinite `Updating` state when no installer remains active and no ready mutex appears. +3. Some reports involve environmental factors (permissions, AV, unusual process state) that can still produce update failures even after duplicate-launch prevention. + +## Validation Strategy + +1. Unit/integration validation for apply guard and recovery transitions. +2. Manual Windows repro validation for all 3 scenarios. +3. Verify startup block behavior when setup/update mutex is active. +4. Confirm logs/telemetry fire for guarded and failure paths. +5. Confirm no regression in normal single-instance background update flow. + +## Out Of Scope (This Fix) + +1. Broad redesign of update UX messaging/modal strategy. +2. Server-side rollout policy changes. +3. Non-Windows update flows. diff --git a/src/vs/code/electron-main/main.ts b/src/vs/code/electron-main/main.ts index eb7e0193373b80..e3db1710a9fdf1 100644 --- a/src/vs/code/electron-main/main.ts +++ b/src/vs/code/electron-main/main.ts @@ -501,9 +501,17 @@ class CodeMain { } try { + const setupMutexName = `${productService.win32MutexName}setup`; const updatingMutexName = `${productService.win32MutexName}-updating`; const mutex = await import('@vscode/windows-mutex'); - return mutex.isActive(updatingMutexName); + const setupActive = mutex.isActive(setupMutexName); + const updatingActive = mutex.isActive(updatingMutexName); + + if (setupActive || updatingActive) { + console.info(`Detected active setup mutex during startup. setupActive=${setupActive}, updatingActive=${updatingActive}`); + } + + return setupActive || updatingActive; } catch (error) { console.error('Failed to check Inno Setup mutex:', error); return false; diff --git a/src/vs/platform/update/electron-main/updateService.win32.ts b/src/vs/platform/update/electron-main/updateService.win32.ts index da4c875845a5d1..84b5d7b6ee4db7 100644 --- a/src/vs/platform/update/electron-main/updateService.win32.ts +++ b/src/vs/platform/update/electron-main/updateService.win32.ts @@ -41,6 +41,20 @@ interface IAvailableUpdate { updateProcess?: ChildProcess; } +interface IWindowsMutexModule { + isActive(mutexName: string): boolean; +} + +type IUpdateWin32ApplyBreadcrumbClassification = { + owner: 'joaomoreno'; + reason: { + classification: 'SystemMetaData'; + purpose: 'FeatureInsight'; + comment: 'Reason for Win32 update apply guard/poll transition.'; + }; + comment: 'This is used to understand Win32 updater setup-coordination transitions.'; +}; + let _updateType: UpdateType | undefined = undefined; function getUpdateType(): UpdateType { if (typeof _updateType === 'undefined') { @@ -315,6 +329,39 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun await Promise.all(promises); } + private async getWindowsMutex(context: string): Promise { + try { + return await import('@vscode/windows-mutex'); + } catch (error) { + this.logService.error(`update#${context}: failed to import windows mutex module`, error); + this.logApplyBreadcrumb('mutex_import_failed'); + return undefined; + } + } + + private isInstallerMutexActive(mutex: IWindowsMutexModule): boolean { + if (!this.productService.win32MutexName) { + return false; + } + + const setupMutexName = `${this.productService.win32MutexName}setup`; + const updatingMutexName = `${this.productService.win32MutexName}-updating`; + return mutex.isActive(setupMutexName) || mutex.isActive(updatingMutexName); + } + + private isReadyMutexActive(mutex: IWindowsMutexModule): boolean { + if (!this.productService.win32MutexName) { + return false; + } + + const readyMutexName = `${this.productService.win32MutexName}-ready`; + return mutex.isActive(readyMutexName); + } + + private logApplyBreadcrumb(reason: 'mutex_import_failed' | 'installer_already_running' | 'resume_installer_missing' | 'poll_timeout' | 'recovery_with_active_installer'): void { + this.telemetryService.publicLog2<{ reason: string }, IUpdateWin32ApplyBreadcrumbClassification>('update:win32ApplyBreadcrumb', { reason }); + } + protected override async doApplyUpdate(): Promise { if (this.state.type !== StateType.Downloaded) { return Promise.resolve(undefined); @@ -328,54 +375,67 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun const explicit = this.state.explicit; this.setState(State.Updating(update)); - const cachePath = await this.cachePath; - const sessionEndFlagPath = path.join(cachePath, 'session-ending.flag'); - const cancelFilePath = path.join(cachePath, `cancel.flag`); - try { - await unlink(cancelFilePath); - } catch { - // ignore + const mutex = await this.getWindowsMutex('doApplyUpdate'); + if (!mutex) { + this.setState(State.Idle(getUpdateType(), 'Unable to coordinate update setup state')); + return; } - const progressFilePath = path.join(cachePath, `update-progress`); - try { - await unlink(progressFilePath); - } catch { - // ignore + const installerAlreadyRunning = this.isInstallerMutexActive(mutex); + if (installerAlreadyRunning) { + this.logService.info('update#doApplyUpdate: installer already running, skipping setup spawn'); + this.logApplyBreadcrumb('installer_already_running'); } - this.availableUpdate.updateFilePath = path.join(cachePath, `CodeSetup-${this.productService.quality}-${update.version}.flag`); - this.availableUpdate.cancelFilePath = cancelFilePath; - - await pfs.Promises.writeFile(this.availableUpdate.updateFilePath, 'flag'); - const child = spawn(this.availableUpdate.packagePath, - [ - '/verysilent', - '/log', - `/update="${this.availableUpdate.updateFilePath}"`, - `/progress="${progressFilePath}"`, - `/sessionend="${sessionEndFlagPath}"`, - `/cancel="${cancelFilePath}"`, - '/nocloseapplications', - '/mergetasks=runcode,!desktopicon,!quicklaunchicon' - ], - { - detached: true, - stdio: ['ignore', 'ignore', 'ignore'], - windowsVerbatimArguments: true + const cachePath = await this.cachePath; + const progressFilePath = path.join(cachePath, `update-progress`); + let spawnedInstaller = false; + if (!installerAlreadyRunning) { + const sessionEndFlagPath = path.join(cachePath, 'session-ending.flag'); + const cancelFilePath = path.join(cachePath, 'cancel.flag'); + try { + await unlink(cancelFilePath); + } catch { + // ignore } - ); - // Track the process so we can cancel it if needed - this.availableUpdate.updateProcess = child; + try { + await unlink(progressFilePath); + } catch { + // ignore + } - child.once('exit', () => { - this.availableUpdate = undefined; - this.setState(State.Idle(getUpdateType())); - }); + this.availableUpdate.updateFilePath = path.join(cachePath, `CodeSetup-${this.productService.quality}-${update.version}.flag`); + this.availableUpdate.cancelFilePath = cancelFilePath; + + await pfs.Promises.writeFile(this.availableUpdate.updateFilePath, 'flag'); + const child = spawn(this.availableUpdate.packagePath, + [ + '/verysilent', + '/log', + `/update="${this.availableUpdate.updateFilePath}"`, + `/progress="${progressFilePath}"`, + `/sessionend="${sessionEndFlagPath}"`, + `/cancel="${cancelFilePath}"`, + '/nocloseapplications', + '/mergetasks=runcode,!desktopicon,!quicklaunchicon' + ], + { + detached: true, + stdio: ['ignore', 'ignore', 'ignore'], + windowsVerbatimArguments: true + } + ); - const readyMutexName = `${this.productService.win32MutexName}-ready`; - const mutex = await import('@vscode/windows-mutex'); + // Track the process so we can cancel it if needed + this.availableUpdate.updateProcess = child; + spawnedInstaller = true; + + child.once('exit', () => { + this.availableUpdate = undefined; + this.setState(State.Idle(getUpdateType())); + }); + } // Poll for progress and ready mutex (timeout after 30 minutes) const pollTimeoutMs = 30 * 60 * 1000; @@ -387,27 +447,33 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun const poll = async () => { while (this.state.type === StateType.Updating && !token.isCancellationRequested) { - if (mutex.isActive(readyMutexName)) { + if (this.isReadyMutexActive(mutex)) { this.setState(State.Ready(update, explicit, this._overwrite)); return; } + if (!spawnedInstaller && !this.isInstallerMutexActive(mutex)) { + this.logService.warn('update#doApplyUpdate: resumed update failed because installer is no longer active and update is not ready'); + this.logApplyBreadcrumb('resume_installer_missing'); + this.setState(State.Idle(getUpdateType(), 'Update did not complete successfully')); + return; + } + if (Date.now() - pollStartTime > pollTimeoutMs) { this.logService.warn('update#doApplyUpdate: polling timed out waiting for update to be ready'); + this.logApplyBreadcrumb('poll_timeout'); this.setState(State.Idle(getUpdateType(), 'Update did not complete within expected time')); return; } try { const progressContent = await readFile(progressFilePath, 'utf8'); - if (!token.isCancellationRequested) { - const [currentStr, maxStr] = progressContent.split(','); - const currentProgress = parseInt(currentStr, 10); - const maxProgress = parseInt(maxStr, 10); - if (!isNaN(currentProgress) && !isNaN(maxProgress) && this.state.type === StateType.Updating) { - if (this.state.currentProgress !== currentProgress || this.state.maxProgress !== maxProgress) { - this.setState(State.Updating(update, currentProgress, maxProgress)); - } + const [currentStr, maxStr] = progressContent.split(','); + const currentProgress = parseInt(currentStr, 10); + const maxProgress = parseInt(maxStr, 10); + if (!isNaN(currentProgress) && !isNaN(maxProgress) && this.state.type === StateType.Updating) { + if (this.state.currentProgress !== currentProgress || this.state.maxProgress !== maxProgress) { + this.setState(State.Updating(update, currentProgress, maxProgress)); } } } catch { @@ -539,6 +605,12 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun this.availableUpdate = { packagePath }; this.setState(State.Downloaded(update, true, false)); + const mutex = await this.getWindowsMutex('_applySpecificUpdate'); + if (mutex && this.isInstallerMutexActive(mutex)) { + this.logService.info('update#_applySpecificUpdate: installer already running during recovery, not spawning a second setup'); + this.logApplyBreadcrumb('recovery_with_active_installer'); + } + if (fastUpdatesEnabled && this.productService.target === 'user') { this.doApplyUpdate(); } else {