Skip to content
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

First SIGINT does not cause process to exit #3657

Closed
3 of 11 tasks
bchabrier opened this issue Nov 3, 2021 · 3 comments · Fixed by #3760
Closed
3 of 11 tasks

First SIGINT does not cause process to exit #3657

bchabrier opened this issue Nov 3, 2021 · 3 comments · Fixed by #3760
Projects

Comments

@bchabrier
Copy link

Is your problem within Home Assistant (Core or Z-Wave JS Integration)?

NO, my problem is NOT within Home Assistant or the ZWave JS integration

Is your problem within ZWaveJS2MQTT?

NO, my problem is NOT within ZWaveJS2MQTT

Checklist

Describe the bug

What causes the bug?

SIGINT is caught in zwave-js by a handler that destroys the driver, but does not call process.exit.

What do you observe?

The first ^C does not kill the process. Looking at the traces, it does the cleanup on the driver, but does not exit. You need to press ^C a second time to have the process really killed.

This can be an issue when the process is a subprocess (e.g. of a shell command). Indeed, the first ^C will kill the parent process, and the zwave process will continue as a zombie.

What did you expect to happen?

The handler should also call process.exit(), so that the process exits on the first ^C.

Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '...'
  3. Scroll down to '...'
  4. See error

Device information

Manufacturer:
Model name:
Node ID in your network:

How are you using node-zwave-js?

  • zwavejs2mqtt Docker image (latest)
  • zwavejs2mqtt Docker image (dev)
  • zwavejs2mqtt Docker manually built (please specify branches)
  • ioBroker.zwave2 adapter (please specify version)
  • HomeAssistant zwave_js integration (please specify version)
  • pkg
  • node-red-contrib-zwave-js (please specify version, double click node to find out)
  • Manually built from GitHub (please specify branch)
  • Other (please describe)

Which branches or versions?

version:
node-zwave-js branch: 8.4.1
zwavejs2mqtt branch:

Did you change anything?

no

If yes, what did you change?

No response

Did this work before?

Don't know, this is a new device

If yes, where did it work?

No response

Attach Driver Logfile

16:52:37.665 DRIVER ███████╗ ██╗ ██╗ █████╗ ██╗ ██╗ ███████╗ ██╗ ███████╗
╚══███╔╝ ██║ ██║ ██╔══██╗ ██║ ██║ ██╔════╝ ██║ ██╔════╝
███╔╝ ██║ █╗ ██║ ███████║ ██║ ██║ █████╗ █████╗ ██║ ███████╗
███╔╝ ██║███╗██║ ██╔══██║ ╚██╗ ██╔╝ ██╔══╝ ╚════╝ ██ ██║ ╚════██║
███████╗ ╚███╔███╔╝ ██║ ██║ ╚████╔╝ ███████╗ ╚█████╔╝ ███████║
╚══════╝ ╚══╝╚══╝ ╚═╝ ╚═╝ ╚═══╝ ╚══════╝ ╚════╝ ╚══════╝
2021-11-03T15:52:37.657Z DRIVER
16:52:37.702 DRIVER version 8.4.1
2021-11-03T15:52:37.702Z DRIVER version 8.4.1
16:52:37.711 DRIVER
2021-11-03T15:52:37.711Z DRIVER
16:52:37.718 DRIVER starting driver...
2021-11-03T15:52:37.718Z DRIVER starting driver...
16:52:37.847 DRIVER opening serial port /dev/ttyACM0
2021-11-03T15:52:37.847Z DRIVER opening serial port /dev/ttyACM0
16:52:43.431 DRIVER serial port opened
2021-11-03T15:52:43.431Z DRIVER serial port opened
16:52:43.444 SERIAL » [NAK] (0x15)
2021-11-03T15:52:43.443Z SERIAL » [NAK] (0x15)
16:52:45.170 DRIVER loading configuration...
2021-11-03T15:52:45.169Z DRIVER loading configuration...
16:52:45.263 CONFIG version 8.4.1
2021-11-03T15:52:45.262Z CONFIG version 8.4.1
================= ^C typed here ====================================
16:52:52.200 DRIVER destroying driver instance...
2021-11-03T15:52:52.199Z DRIVER destroying driver instance...
================== process it no exited ================================

@zwave-js-bot zwave-js-bot added this to Needs triage in Triage Nov 3, 2021
@AlCalzone
Copy link
Member

I'll need some steps to reproduce. If I run the driver using yarn ts test/run.ts in the repository, the first CTRL+C does destroy the driver and exit the process:
sigint

@bchabrier
Copy link
Author

Here is short program to reproduce:

pi@raspberrypi ~/domoja/modules/openzwave $ cat test-zwave.ts
import { Driver } from "zwave-js";

const driver = new Driver('/dev/ttyACM0', {
    logConfig: {
        enabled: true,
        level: "silly",
    }
});
driver.on("error", (err) => {console.error(err)});
driver.start().catch(reason => {console.log(reason)});

console.log('Starting loop...');
setInterval(() => {console.log('looping')}, 1000);

When you run it, the first ^C destroys the driver, the second ^C is needed to stop the program.

pi@raspberrypi ~/domoja/modules/openzwave $ ts-node test-zwave.ts
21:15:35.553 DRIVER   ███████╗ ██╗    ██╗  █████╗  ██╗   ██╗ ███████╗             ██╗ ███████╗
                      ╚══███╔╝ ██║    ██║ ██╔══██╗ ██║   ██║ ██╔════╝             ██║ ██╔════╝
                        ███╔╝  ██║ █╗ ██║ ███████║ ██║   ██║ █████╗   █████╗      ██║ ███████╗
                       ███╔╝   ██║███╗██║ ██╔══██║ ╚██╗ ██╔╝ ██╔══╝   ╚════╝ ██   ██║ ╚════██║
                      ███████╗ ╚███╔███╔╝ ██║  ██║  ╚████╔╝  ███████╗        ╚█████╔╝ ███████║
                      ╚══════╝  ╚══╝╚══╝  ╚═╝  ╚═╝   ╚═══╝   ╚══════╝         ╚════╝  ╚══════╝
21:15:35.583 DRIVER   version 8.4.1
21:15:35.587 DRIVER
21:15:35.589 DRIVER   starting driver...
21:15:35.686 DRIVER   opening serial port /dev/ttyACM0
Starting loop...
21:15:35.767 DRIVER   serial port opened
21:15:35.774 SERIAL » [NAK]                                                                   (0x15)
looping
21:15:37.296 DRIVER   loading configuration...
21:15:37.314 CONFIG   version 8.4.1
looping
looping
...
...
looping
looping
looping
looping
^C21:16:19.779 DRIVER   destroying driver instance...
looping
looping
looping
looping
looping
looping
^C
pi@raspberrypi ~/domoja/modules/openzwave $ 

Note that if the driver is not started correctly or has error, then the handler on SIGINT is not active, hence the first ^C will indeed kill the process.

Hope it helps!

@zwave-js-assistant zwave-js-assistant bot added the stale 💤 This issue seems to have gone stale. Interact with it to keep it open label Nov 11, 2021
@zwave-js-assistant
Copy link

This issue has not seen any recent activity and was marked as "stale 💤".
Closing for housekeeping purposes... 🧹

Feel free to reopen if the issue persists.

Triage automation moved this from Needs triage to Closed Nov 14, 2021
@AlCalzone AlCalzone removed waiting for info ⏳ stale 💤 This issue seems to have gone stale. Interact with it to keep it open labels Nov 14, 2021
@AlCalzone AlCalzone reopened this Nov 14, 2021
Triage automation moved this from Closed to Needs triage Nov 14, 2021
@AlCalzone AlCalzone moved this from Needs triage to Done but not merged in Triage Nov 19, 2021
Triage automation moved this from Done but not merged to Closed Nov 21, 2021
bchabrier added a commit to bchabrier/domoja that referenced this issue Dec 12, 2021
bchabrier added a commit to bchabrier/domoja that referenced this issue Dec 12, 2021
bchabrier added a commit to bchabrier/domoja that referenced this issue Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Triage
Closed
Development

Successfully merging a pull request may close this issue.

2 participants