Skip to content

fix(cli): avoid Windows update check crash - #455

Closed
Devin Rousso (dcrousso) wants to merge 1 commit into
mainfrom
fix-42402
Closed

fix(cli): avoid Windows update check crash#455
Devin Rousso (dcrousso) wants to merge 1 commit into
mainfrom
fix-42402

Conversation

@dcrousso

Copy link
Copy Markdown

fetch() can race with process.exit() and trigger a libuv assertion on Windows with Node 24

use a nonpersistent https connection for the update check

fixes microsoft/playwright#42402

`fetch()` can race with `process.exit()` and trigger a libuv assertion on Windows with Node 24

use a nonpersistent `https` connection for the update check
Comment thread playwright-cli.js
}

async function fetchLatestVersion() {
const agent = new https.Agent({ keepAlive: false });

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's specifically how the crash happens:

  1. the fetch below completes, but Undici can still have delayed work queued on Node’s worker task scheduler
  2. --version will then immediately calls process.exit(0)
  3. Node starts shutdown and closes the scheduler’s uv_async_t handle
  4. the queued fetch work races with shutdown and calls uv_async_send() on that closing handle
  5. Windows libuv hits assert(!(handle->flags & UV_HANDLE_CLOSING)) and aborts with 0xC0000409

this is why the version appears before the crash

NO_UPDATE_NOTIFIER=1 avoids fetch(), so there is no delayed work to race with process.exit()

technically it would probably be better to replace some/all of the process.exit(0) in playwright-core to allow for a more graceful shutdown

but this is also arguably correct since there's no need to keep the connection alive any longer

note there is an upstream fix for this nodejs/node#61999 but it has not been put into any release yet and i think this affects as far back as Node 23

@yury-s Yury Semikhatsky (yury-s) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it help with the issue?

Comment thread playwright-cli.js
}

async function fetchLatestVersion() {
const agent = new https.Agent({ keepAlive: false });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a new agent?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i felt this was clearer than relying on the default/global agent implicitly having keepAlive: false

@dcrousso

Copy link
Copy Markdown
Author

closing in favor of the upstream fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: playwright-cli v0.1.18 crashes on startup on Windows with libuv assertion (v0.1.17 works)

2 participants