-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Node script isn't killable if importing chromium #19418
Comments
It doesn't seem to be playwright specific, the following code also fails to exit on Ctrl+C: process.on('SIGINT', () => {
process.exit(2);
})
for (let i = 0; i < 1_000_000; i += 1) {
console.log(i);
} It doesn't hang for some other imports though, so I wonder what is that specific to playwright package that makes node ignore sigint. |
Testing both of these on node v18.12.1 and control-c does properly exit the script – although in the rest repo (the scenario using playwright) it takes about 7-9 seconds to exit. In your version with the It would be interesting to see what about playwright is causing that to happen – especially since there is no code being executed – just the import. |
This issue is informative: nodejs/node#9050 So this isn't a bug per se but it seems that there is a process listener somewhere in the code that could be handled better. |
Yeah, I am not aware of any sigint handlers that playwright would add during its initialization (we use some but all of them are related to child process launches), so my hunch is that a third-party dependency might be adding such a listener. |
Ok, the culprit is
that module unconditionally installs signal handlers during its initialization . This is a very interesting design choice but given that it only causes a problem when you have a toplevel tight loop fixing it will probably be a lower priority for us and I don't see an easy work around at this point. Let me bring this is up in the team meeting tomorrow. |
Thanks for the quick and thorough investigation! That's interesting and I see that repo has some issues about this, too. My use case is that I use playwright in this package where the workflow is:
So hence why I'm using a toplevel loop. Thanks again for looking into it! |
We've decided in the team meeting that it would make sense for playwright to fix the lock file support and make it not depend on signal-exit code. |
Very exciting – thanks for your time in working on it. |
This patch vendors the https://github.com/moxystudio/node-proper-lockfile project we rely to manage Playwright Registry. The reason to vender is the following upstream issue that didn't get resolved in almost a month: moxystudio/node-proper-lockfile#111 Follow-up will apply the fix for the issue to the vendored file. NOTE: this patch also inlines all code in a single file. References microsoft#19418
This patch vendors the https://github.com/moxystudio/node-proper-lockfile project we rely to manage Playwright Registry. The reason to vender is the following upstream issue that didn't get resolved in almost a month: moxystudio/node-proper-lockfile#111 Follow-up will apply the fix for the issue to the vendored file. NOTE: this patch also inlines all code in a single file. References microsoft#19418
This patch vendors the https://github.com/moxystudio/node-proper-lockfile project we rely to manage Playwright Registry. The reason to vender is the following upstream issue that didn't get resolved in almost a month: moxystudio/node-proper-lockfile#111 Follow-up will apply the fix for the issue to the vendored file. NOTE: this patch also inlines all code in a single file. References #19418
Subscription to SIGINT handler removes default handler. Fixes microsoft#19418
Subscription to SIGINT handler removes default handler. Fixes #19418
Thanks for your work on this @aslushnikov @yury-s @dgozman ! I'm looking forward to using it in the next release |
Context:
System:
Binaries:
Languages:
npmPackages:
Code Snippet
Reproduction repo: https://github.com/mhkeller/playwright-process-bug
Describe the bug
If you
import { chromium } from 'playwright'
, the parent node process can no longer be exited with control-c.Steps to reproduce
node index.js
To fix it, comment out the chromium import line in
my-module.js
.playwright-bug.mov
The text was updated successfully, but these errors were encountered: