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

Properly shutdown the installer's child process #3260

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Properly shutdown the installer's child process #3260

merged 1 commit into from
Jan 18, 2021

Conversation

pschichtel
Copy link
Contributor

This is the proper solution for #3251. Without this change, shutting down the server only works by pressing ctrl-c while having a terminal open with the stdin of the child process (actual MC server), since the child is started with inheritIO(). When the stdin is not kept open, which would be normal for a swarm deployment, before this commit there was actually no way of cleanly shutting down the server, because PID1 received the INT/TERM signal, started its shutdown hook which then waits forever for the child to exit, since the child does not know it's supposed to exit. Now it does, I tested this in a few scenarios.

Again a tiny change to work better in container environments.

This is the proper solution for #3251. Without this change, shutting down the server only works by pressing ctrl-c while having a terminal open with the stdin of the child process (actual MC server), since the child is started with `inheritIO()`. When the stdin is not kept open, which would be normal for a swarm deployment, before this commit there was actually no way of cleanly shutting down the server, because PID1 received the INT/TERM signal, started its shutdown hook which then waits forever for the child to exit, since the child does not know it's supposed to exit. Now it does, I tested this in a few scenarios.
@Zidane Zidane merged commit 592754e into SpongePowered:api-8 Jan 18, 2021
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.

2 participants