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

Fixes to make Docker containers shutdown properly #1115

Merged
merged 2 commits into from Jul 5, 2018

Conversation

Tydus
Copy link
Contributor

@Tydus Tydus commented Jul 4, 2018

Making a dockerized daemon receive SIGTERM could be very tricky:
firstly, CMD should not use the "shell form", otherwise a "sh -c" process will possess the PID 1 therefore no one can receive the SIGTERM sent from "docker stop";
second, the bitsharesentry.sh should call $BITSHARESD with exec, i.e. let the daemon take the same PID (i.e. 1). Or in another way, a "trap SIGTERM" trick may be employed to make sure the SIGTERM is forwarded to the actual daemon process.

The second patch is optional. I personally encountered segfaults while sending SIGTERM to witness, however SIGINT works well ever.

See also: https://hynek.me/articles/docker-signals/

@abitmore abitmore requested a review from xeroc July 4, 2018 12:28
@abitmore
Copy link
Member

abitmore commented Jul 4, 2018

@Tydus thanks for the pull request. Please rebase and change the base of this PR to develop branch.

@Tydus Tydus changed the base branch from master to develop July 4, 2018 16:16
@Tydus Tydus changed the base branch from develop to master July 4, 2018 16:16
@Tydus Tydus changed the base branch from master to develop July 4, 2018 16:18
Tydus added 2 commits July 5, 2018 01:20
Making a dockerized daemon receive SIGTERM could be very tricky:
firstly, CMD should not use the "shell form", otherwise a "sh -c" process
will possess the PID 1 therefore no one can receive the SIGTERM sent
from "docker stop";
second, the bitsharesentry.sh should call $BITSHARESD with exec, i.e.
let the daemon take the same PID (i.e. 1). Or in another way, a
"trap SIGTERM" trick may be employed to make sure the SIGTERM is
forwarded to the actual daemon process.
@Tydus
Copy link
Contributor Author

Tydus commented Jul 4, 2018

@abitmore Thanks abit. It looks OK now.

@abitmore
Copy link
Member

abitmore commented Jul 4, 2018

Thanks.

@abitmore abitmore requested a review from ryanRfox July 4, 2018 21:56
@abitmore abitmore merged commit d09a347 into bitshares:develop Jul 5, 2018
@abitmore abitmore removed the request for review from ryanRfox July 21, 2018 21:27
@abitmore abitmore added this to In Development in Feature Release (201808) via automation Jul 21, 2018
@abitmore abitmore moved this from In Development to Done in Feature Release (201808) Jul 21, 2018
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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants