From 1ae90ca0e5716064781f2c71d6dac676ca91cce1 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Fri, 4 Sep 2026 10:24:20 +0900 Subject: [PATCH] Fix Windows launcher race condition On Windows, the target command always runs through a `powershell.exe` launcher, which can take noticeably longer than the ~10-30ms assumed for other platforms' launchers to start executing. The non-fallback path only waited for the `spawn` event before resolving and letting the caller's process exit, so a caller exiting right after `await open(...)` could tear down the still-starting, non-detached launcher before it ran `Start-Process` and actually opened the target app. Reuse the existing fallback-attempt handling, which already waits for the launcher's `close` event, for the Windows path as well. This does not wait for the opened app itself, only for the launcher script. Fixes #298 --- index.js | 13 ++++++++++++- test.js | 48 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/index.js b/index.js index 39f7e2a..1308946 100644 --- a/index.js +++ b/index.js @@ -280,7 +280,18 @@ const baseOpen = async options => { // When we're in a fallback attempt, we need to detect launch failures before trying the next app. // Wait for the close event to check the exit code before unreffing. // The launcher (open/xdg-open/PowerShell) exits quickly (~10-30ms) even on success. - if (isFallbackAttempt) { + // + // On Windows, the actual command always runs inside a `powershell.exe` launcher (see + // above). Unlike `open`/`xdg-open`, this launcher can take noticeably longer than + // ~10-30ms to start executing the encoded script, so waiting only for `'spawn'` isn't + // enough: the promise can resolve, and the caller's process can exit, before + // `Start-Process` has actually run. Since the launcher isn't detached on Windows, + // exiting at that point can tear it down before it launches the target app. Always + // wait for the launcher's own `'close'` event on Windows to avoid this race. This does + // not wait for the *opened app* to close, only for the (fast) launcher script. + const isWindowsLauncher = platform === 'win32' || shouldUseWindowsInWsl; + + if (isFallbackAttempt || isWindowsLauncher) { return new Promise((resolve, reject) => { subprocess.once('error', reject); diff --git a/test.js b/test.js index 500b301..df8b5b5 100644 --- a/test.js +++ b/test.js @@ -139,13 +139,9 @@ test('subprocess is spawned before promise resolves', async t => { t.true(childProcess.pid !== undefined && childProcess.pid !== null); }); -test.serial('app launches resolve before close without fallback', async t => { +const stubSpawnEmittingCloseAfter = delay => { const originalSpawn = childProcess.spawn; - t.teardown(() => { - childProcess.spawn = originalSpawn; - }); - - let closeEmitted = false; + const state = {closeEmitted: false}; childProcess.spawn = () => { // eslint-disable-next-line unicorn/prefer-event-target @@ -155,18 +151,46 @@ test.serial('app launches resolve before close without fallback', async t => { setImmediate(() => { fakeChild.emit('spawn'); setTimeout(() => { - closeEmitted = true; + state.closeEmitted = true; fakeChild.emit('close', 0); - }, 50); + }, delay); }); return fakeChild; }; - const subprocess = await open('index.js', {app: {name: 'stub-app'}}); - t.false(closeEmitted); - t.truthy(subprocess); -}); + return {originalSpawn, state}; +}; + +if (process.platform === 'win32') { + // On Windows the spawned process is always a `powershell.exe` launcher, which can take + // noticeably longer to start executing the encoded command than the ~10-30ms other + // platforms' launchers take. If the promise resolved before the launcher closes (as on + // other platforms), callers that exit right after `await open(...)` could tear down the + // non-detached launcher before it has actually started the target app. + // See https://github.com/sindresorhus/open/issues/298 + test.serial('app launches on Windows resolve only after the launcher closes', async t => { + const {originalSpawn, state} = stubSpawnEmittingCloseAfter(50); + t.teardown(() => { + childProcess.spawn = originalSpawn; + }); + + const subprocess = await open('index.js', {app: {name: 'stub-app'}}); + t.true(state.closeEmitted); + t.truthy(subprocess); + }); +} else { + test.serial('app launches resolve before close without fallback', async t => { + const {originalSpawn, state} = stubSpawnEmittingCloseAfter(50); + t.teardown(() => { + childProcess.spawn = originalSpawn; + }); + + const subprocess = await open('index.js', {app: {name: 'stub-app'}}); + t.false(state.closeEmitted); + t.truthy(subprocess); + }); +} test('fallback to next app when first app does not exist', async t => { // Try nonexistent apps first, then a real app