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.

Sign up for GitHub

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

Jump to bottom

Capture SIGINT, SIGTERM, SIGQUIT signals from terminal in both iCubTelemVizServer and openmctStaticServer and close the later #38

Merged
merged 3 commits into from
Oct 20, 2021
Merged

Capture SIGINT, SIGTERM, SIGQUIT signals from terminal in both iCubTelemVizServer and openmctStaticServer and close the later #38

merged 3 commits into from
Oct 20, 2021

Conversation

nunoguedelha
Copy link
Collaborator

@nunoguedelha nunoguedelha commented Sep 28, 2021 •
edited
Loading

Fixes #41 .
Fixes #42 .

@nunoguedelha nunoguedelha self-assigned this Sep 28, 2021
@nunoguedelha
Capture the SIGINT, SIGTERM, SIGQUIT events sent from the terminal se…
8c96eb8 
…parately to each node.js process

- Sending <CTRL+C> to the main process stdin ('iCubTelemVizServer') is propagated to the 'openmctStaticServer' process.
- Sending any of the three events through the command 'kill' is not propagated from 'iCubTelemVizServer' to 'openmctStaticServer'. Kill has to be sent separately to each process.
@nunoguedelha
Capture properly SIGINT, SIGTERM, SIGQUIT events for closing the open…
a044f2b 
…mct child process

- The previously returned 'ChildProcess' object was the 'npm' process.
- This only allowed killing the child process through CTRL+C input (that
  stdin string would then generate the SIGINT event which would then be
  propagated to the child processes
  npm start
     -> sh -c ". ${NVM_DIR}/nvm.sh; nvm use v14.17.0; node server.js"
         -> node server.js
- Sending a SIGQUIT, SIGTERM or SIGINT from terminal though 'kill -s...'
  to the 'npm start' process, parent of the node.js process wouldn't kill
  the later, and there was no direct way to retrieve the node.js process
  pid programmatically.
- We should retrieve the PID information directly from the child node.js
  process through an IPC channel between child and parent node.js processes.
- For this we add a 4rd stdio entry for the spawned command:
  'spawn(...,{..., stdio: ['pipe','pipe','pipe','ipc']})'.
- We send the node.js child process PID to the parent process via the created
  IPC channel.
- We have to spawn a 'sh runModule.sh' command instead of the 'npm start' as
  npm does not propagate the IPC channel either.
- The sh script 'runModule.sh' holds the same commands as the npm script used to.

Now both node.js processes can be killed through 'kill -s...' command or through
the 'CTRL+C' input.
@nunoguedelha nunoguedelha mentioned this pull request Oct 14, 2021
Closed
3 tasks
@nunoguedelha
Close the 'openmctStaticServer' server and any connections remained open
1533694 
- Check if IPC channel is available from the Node.js child process
  before using it, otherwise we get an error (undefined 'process.send()')
  when we run 'openmctStaticServer' module as a standalone process.
- Track all opened connections in 'openmctStaticServer' module.
- Close the server (stop it from accepting new connections) and close any
  connections remained open.
@nunoguedelha
Copy link
Collaborator Author

Testing the changes

  • Run the application as explained in the repository main README (simulation section).
  • you should get something like:
> [email protected] start /Users/nunoguedelha/dev/yarp-openmct/iCubTelemVizServer
> . ${NVM_DIR}/nvm.sh; nvm use v4.2.2; node ${NODE_DEBUG_OPTION} iCubTelemVizServer.js

Now using node v4.2.2 (npm v2.14.7)
iCub Telemetry server launched!
[INFO] |yarp.os.Port| Port /yarpjs/inertial:i active at tcp://192.168.1.100:10125/
[INFO] |yarp.os.impl.PortCoreInputUnit| Receiving input from /icubSim/inertial to /yarpjs/inertial:i using tcp
[INFO] |yarp.os.Port| Port /yarpjs/left_leg/stateExt:o active at tcp://192.168.1.100:10126/
[INFO] |yarp.os.impl.PortCoreInputUnit| Receiving input from /icubSim/left_leg/stateExt:o to /yarpjs/left_leg/stateExt:o using tcp
[INFO] |yarp.os.Port| Port /yarpjs/camLeftEye:i active at tcp://192.168.1.100:10127/
[INFO] |yarp.os.impl.PortCoreInputUnit| Receiving input from /icubSim/camLeftEye to /yarpjs/camLeftEye:i using tcp
[INFO] |yarp.os.Port| Port /yarpjs/camRightEye:i active at tcp://192.168.1.100:10128/
[INFO] |yarp.os.Port| Port /yarpjs/left_leg/cartesianEndEffectorWrench:i active at tcp://192.168.1.100:10129/
[INFO] |yarp.os.impl.PortCoreInputUnit| Receiving input from /wholeBodyDynamics/left_leg/cartesianEndEffectorWrench:o to /yarpjs/left_leg/cartesianEndEffectorWrench:i using tcp
[INFO] |yarp.os.Port| Port /yarpjs/right_leg/cartesianEndEffectorWrench:i active at tcp://192.168.1.100:10130/
[INFO] |yarp.os.impl.PortCoreInputUnit| Receiving input from /wholeBodyDynamics/right_leg/cartesianEndEffectorWrench:o to /yarpjs/right_leg/cartesianEndEffectorWrench:i using tcp
[INFO] |yarp.os.Port| Port /yarpjs/battery/data:i active at tcp://192.168.1.100:10131/
[INFO] |yarp.os.Port| Port /yarpjs/sysCmdsGenerator/rpc active at tcp://192.168.1.100:10132/
OK
Opem-MCT static server process started.
ICubTelemetry History hosted at http://localhost:8081/history
ICubTelemetry Realtime hosted at ws://localhost:8081/realtime
listening on http://localhost:3000
[OPEN-MCT STATIC SERVER] stdout: Now using node v14.17.0 (npm v6.14.13)

[OPEN-MCT STATIC SERVER] stderr: Debugger listening on ws://127.0.0.1:9229/fe8b8e7c-40ad-4ddf-a9d4-5aac8306e1a7
For help, see: https://nodejs.org/en/docs/inspector

[OPEN-MCT STATIC SERVER] stderr: Debugger attached.

[OPEN-MCT STATIC SERVER] ipc: {"pid":71770}
[OPEN-MCT STATIC SERVER] stdout: iCub Telemetry Visualizer (Open MCT based) hosted at http://localhost:8080

In this example, battery status and the fake framegrabber are not running nor connected.

  • Click on http://localhost:8080, a tab with the visualizer opens in your browser. You should get the below connection logging information
[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION
  • If you click on an iCub Telemetry entry, you should see
[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: NEW CONNECTION!!

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

[OPEN-MCT STATIC SERVER] stdout: CLOSE CONNECTION

These connections are created for retrieving the signal JSON dictionary every time you click on a telemetry entry (this behaviour could be improved later).

  • Kill the process node iCubTelemVizServer.js with a SIGQUIT signal from another terminal: kill -s SIGQUIT <pid>.
Received SIGQUIT ...
Killing PID 71770 with signal SIGQUIT
[OPEN-MCT STATIC SERVER] stdout: Received SIGQUIT ...

[OPEN-MCT STATIC SERVER] stdout: Open-MCT vVisualizer Server closed. No further requests accepted. Refreshing the visualizer web page will fail.

[OPEN-MCT STATIC SERVER] close: 0
  • As only the openmctStaticServer termination is handled in this PR, the process node server.js should have disappeared from the list of running processes but the process node iCubTelemVizServer.js should still be runing.

Stopping the application with SIGINT or by typing <CTRL+C> gives the same result with a slightly different terminal output.

@nunoguedelha nunoguedelha marked this pull request as ready for review October 14, 2021 05:35
@nunoguedelha nunoguedelha mentioned this pull request Oct 14, 2021
Closed
@nunoguedelha nunoguedelha changed the title Close the servers properly when interrupting from the terminal via SIGINT (CTRL+C) or SIGTERM signals Capture SIGINT, SIGTERM, SIGQUIT signals from terminal in both iCubTelemVizServer and openmctStaticServer and close the later Oct 14, 2021
@nunoguedelha nunoguedelha merged commit 1533694 into main Oct 20, 2021
@nunoguedelha nunoguedelha deleted the fix/servers-closure branch October 20, 2021 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@S-Dafarra S-Dafarra S-Dafarra approved these changes

Assignees

@nunoguedelha nunoguedelha

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@nunoguedelha @S-Dafarra