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

Double SIGINT on openmctStaticServer process still triggering uncaught SIGINT event #57

Closed
Tracked by #40
nunoguedelha opened this issue Nov 2, 2021 · 5 comments · Fixed by #56
Closed
Tracked by #40
Assignees
Labels
bug Something isn't working

Comments

@nunoguedelha
Copy link
Collaborator

nunoguedelha commented Nov 2, 2021

The issue fixed in in #44 (refer to #44 (comment)_) is still present.

  • When interrupting the app sending <CTRL+C> from the terminal stdin, a group SIGINT event is generated and sent to all running processes in the group (node icubTelemVizServer.js and node server.js).
  • As a consequence two SIGINT events are captured in the child process node server.js: the one received directly through the group SIGINT, and the one forwarded from the parent process through a kill -s INT command.
  • The double event processing causes a premature, potentially problematic, closure of the child process and underlying tasks, and this is due to the fact that the second event is processed by the default SIGINT handler.

Solution

  • We stop listening to the SIGINT events while we wait for the server closure to be completed. Such wait should be limited by a timeout for avoidning a lock in case of an unexpected issue delays the server closure indefinitely.

It was happening occasionally, and commenting the removal of the timeout and the idle listener implemented in 75b6fe8 would avoid the issue, but this has to be properly fixed.

@nunoguedelha
Copy link
Collaborator Author

Even if the handleTermination listener can be called mutliple times, or if the idle listener inhibit2ndSIGINT is added for inhibiting the second SIGINT event, if the server closure is fast enough and the node process's event loop turns out empty, the process exits and as a result:

  • The second SIGINT event is received and processed by the direct parent process, which is the sh runModule.sh.
  • The SIGINT in that process is processed by the default handler, generating the "Uncaught Signal Exit".

@nunoguedelha
Copy link
Collaborator Author

A simple test can verify that explanation

  • The callbackhandleTermination is added with prependListener method instead of prependOnceListener.
  • A counter placed in the callback counts a single function call before the process exits (counter returned with the log message).
  • If we add a 5 seconds delay via a setTimeout, the counter returns 2 counts.

@nunoguedelha
Copy link
Collaborator Author

We can fix the problem by simply running the Open-MCT Visualizer Server as a background process, detached from the shell command sh runModule. This is easily done by adding "&" to the line node server.js:

. ${NVM_DIR}/nvm.sh
nvm use v14.17.0
node server.js &

Although the node process is detached, its stdin, stdout, stderr are still connected to the shell process as well as the originating iCubTelemetry server process.

@nunoguedelha
Copy link
Collaborator Author

nunoguedelha commented Nov 3, 2021

Fixed in #56 .

@nunoguedelha
Copy link
Collaborator Author

In order to capture the actual exit code from the open-mct child process, we should send the process object to the telemetry server parent project. This is low priority and can be done later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant