carboneio / carbone

Fast and simple report generator, from JSON to pdf, xslx, docx, odt...
https://carbone.io
Other
1.3k stars 190 forks source link

[Bug Report]: CTRL+c does not kill the process #181

Open IlyaRazuvaev opened 1 year ago

IlyaRazuvaev commented 1 year ago

Environment Carbone Version: 3.5.5 Node Version: 18.13.0 Desktop OS: tested on "darwin" (Mac), "win32" (Windows 10).

Related issue: https://github.com/carboneio/carbone/issues/179#issuecomment-1480739440

CTRL+c does not kill the process in new version of carbone.

genadis commented 1 year ago

The issue is due to this addition: https://github.com/carboneio/carbone/blob/master/lib/converter.js#L659 :

['SIGINT', 'SIGHUP', 'SIGQUIT'].forEach(function (signal) {
  process.on(signal, function () {
    converter.exit();
  });
});

According to Nodejs docs, adding listener to those events override default OS behaviour causing the process to hang and never exit: https://nodejs.org/docs/latest-v18.x/api/process.html#signal-events

changing the code to below fixes the issue, and does not break any custom handling for apps that use carbone and require their own custom handling of the signals:

['SIGINT', 'SIGHUP', 'SIGQUIT'].forEach(function (signal) {
    const cleanup = () => {
        converter.exit(() => {
            process.removeListener(signal, cleanup);
            process.kill(process.pid, signal);
        });
    }
    process.on(signal, cleanup);
});