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

launcher: When terminated, ensure child processes are cleaned up #75

Merged
merged 1 commit into from
Mar 18, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Mar 18, 2019

Relates to #8.

Overview

The child process(es) were cleaned up in the event of Ctrl-C (SIGINT), but not when the launcher was killed by the kill command (SIGTERM).

@rvl rvl added this to the Basic Launcher milestone Mar 18, 2019
@rvl rvl self-assigned this Mar 18, 2019
@rvl rvl requested review from Anviking and KtorZ March 18, 2019 05:30
@KtorZ
Copy link
Member

KtorZ commented Mar 18, 2019

@rvl Have you tested this 🤔 ?
When I was finalizing the PR on the launcher, I spent quite some time trying to install some signal handlers without much success for the SIG_KILL. Others, and in particular SIG_INT are handled properly by the process library and the bracket we use, making sure to clean up children as the main launcher exits.

However, I doesn't seem possible to catch a SIG_KILL and do anything with it. I believe that's part of the semantic of SIG_KILL actually, it just terminates the process, immediately.

cf:

$ ps -ef | grep cardano
ktorz     7021  5284  0 08:35 pts/17   00:00:00 /home/ktorz/Documents/IOHK/cardano-wallet/.stack-work/install/x86_64-linux/lts-13.8/8.6.3/bin/cardano-wallet-launcher
ktorz     7053  7021 67 08:35 pts/17   00:00:12 cardano-http-bridge start --port 8080
ktorz     7121  7021 93 08:35 pts/17   00:00:16 cardano-wallet-server --wallet-server-port 8090 --http-bridge-port 8080 --network mainnet

$ kill 7021 // or kill -9 7021

$ ps -ef | grep cardano
ktorz     7053  2225 85 08:35 pts/17   00:00:58 cardano-http-bridge start --port 8080
ktorz     7121  2225 94 08:35 pts/17   00:01:04 cardano-wallet-server --wallet-server-port 8090 --http-bridge-port 8080 --network mainnet

@rvl
Copy link
Contributor Author

rvl commented Mar 18, 2019

Yes I've tested it. You can't do much about SIGKILL (kill -9).

But SIGTERM (kill with the default signal) was not working until I made this change. SIGTERM will be a common way of stopping the wallet.

@KtorZ
Copy link
Member

KtorZ commented Mar 18, 2019

@rvl May you specify the manual steps you use to test this? It doesn't seem to work for me using the default signal for kill

My bad. I didn't check the branch out actually.

@KtorZ KtorZ merged commit 653d8ec into master Mar 18, 2019
@KtorZ KtorZ deleted the rvl/8/sigterm-handler branch March 18, 2019 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants