From 4b9ebd841c663bec1b09fdd80185e21682a96337 Mon Sep 17 00:00:00 2001 From: Tyler Murphy Date: Wed, 21 Jul 2021 17:09:13 -0400 Subject: [PATCH] Make sure shutdown signals aren't trapped forever Right now, it seems like this library is preventing the `SIGINT` (ctrl-C) signal from shutting down my project. I made these changes using github's edit button, so I haven't tested/linted anything, and there could be mistakes. See this comment for an explanation: https://github.com/berstend/puppeteer-extra/issues/467#issuecomment-864191175. Here are the comment's contents at the time of writing, in case the link stops working: I've run into problems like this before, where a `process.on` listener traps `SIGINT` every time it's sent. Reproduce the problem with this script. Run it and try hitting `ctrl-C` repeatedly. It'll be trapped every time and the process won't exit. ```node process.on(`SIGINT`, () => console.log(`caught SIGINT`)) setInterval(() => {}, 1e3) // do work to prevent immediate exit ``` I've fixed this a couple ways in the past. 1. Use `process.once`, and after cleanup work is complete, print a message saying something like "send SIGINT again to exit". 2. Re-emit the kill signal after cleanup work is complete. Option 2 seems like a better fit in this case, and it works better in conjunction with other, unrelated cleanup scripts with their own `SIGINT` listeners. Here's what it looks like: ```node process.once(`SIGINT`, () => { console.log(`caught SIGINT`) process.kill(process.pid, `SIGINT`) // send `SIGINT` back to self }) setInterval(() => {}, 1e3) // do work ``` I tried replacing [the `puppeteer-extra-plugin` `process.on` listeners in the `index` file](https://github.com/berstend/puppeteer-extra/blob/0049d6010311505f27e7f3be804bb198e2c09aa2/packages/puppeteer-extra-plugin/src/index.ts#L506-L519) with `process.once` listeners as described, and it seems to fix the problem for me. For example: ```node if (this.onClose) { if (opts.options.handleSIGINT !== false) { process.once('SIGINT', () => { this.onClose() process.kill(process.pid, `SIGINT`) }); } ... ``` **Note:** I haven't tested this against the other test cases in this thread, but it solved my problem. Maybe others can see if it works for them, too. --- packages/node/src/relay/index.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/node/src/relay/index.ts b/packages/node/src/relay/index.ts index 74ec026c2..26105e6f1 100644 --- a/packages/node/src/relay/index.ts +++ b/packages/node/src/relay/index.ts @@ -44,7 +44,13 @@ export default class RelayClient extends BaseSession { await this.disconnect() } - process.on('SIGTERM', _gracefulDisconnect) - process.on('SIGINT', _gracefulDisconnect) + process.once('SIGTERM', async () => { + await _gracefulDisconnect() + process.kill(process.pid, `SIGTERM`) + }) + process.once('SIGINT', async () => { + await _gracefulDisconnect() + process.kill(process.pid, `SIGINT`) + }) } }