fix: handle SIGHUP in launcher-patcher to prevent exits before updating#1641
Merged
fix: handle SIGHUP in launcher-patcher to prevent exits before updating#1641
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the launcher-patcher prematurely exits due to SIGHUP signals from terminal multiplexers like screen or tmux and ensures that the patching process continues by introducing a timeout mechanism.
- Added a signal handler to ignore SIGHUP signals.
- Introduced a 5-second timeout when waiting for the launcher process to terminate.
- Replaced direct System.out.printf calls with a helper printf method for consistent logging.
Comments suppressed due to low confidence (2)
launcher/patcher/src/main/java/eu/cloudnetservice/launcher/patcher/CloudNetLauncherPatcher.java:52
- [nitpick] Consider including the timeout value (5 seconds) in the error message to provide clearer context during troubleshooting.
printf(System.err, "Launcher process did not terminate in time, running patcher anyway");
launcher/patcher/src/main/java/eu/cloudnetservice/launcher/patcher/CloudNetLauncherPatcher.java:36
- The code uses sun.misc.Signal from an internal API; consider verifying that this approach meets your long-term compatibility and maintainability requirements, or explore alternatives if available.
Signal.handle(new Signal("HUP"), SignalHandler.SIG_IGN);
derklaro
approved these changes
May 26, 2025
GiantTreeLP
pushed a commit
to GiantTreeLP/CloudNet
that referenced
this pull request
Jun 7, 2025
…ng (CloudNetService#1641) ### Motivation The launcher-patcher tries to update the launcher.jar after the node & launcher exit. This process did not work when `screen` or `tmux` was used to start the launcher due to the fact that they send a `SIGHUG` signal and the JVM handles the signal and stops the still running (waiting) launcher-patcher. This required users to download the launcher themselves when we did changes to the launcher. ### Modification Introduced a handler for `SIGHUP` in the launcher-patcher that ignores the signal. Furthermore, this introduces a 5 second timeout when waiting for the launcher to exit before running the patcher as the launcher should not take that long to stop and to prevent replacing the next running launcher instance getting replaced while running. ### Result Launcher updates are properly working when using `screen` or `tmux` --------- Co-authored-by: Pasqual Koschmieder <git@derklaro.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The launcher-patcher tries to update the launcher.jar after the node & launcher exit. This process did not work when
screenortmuxwas used to start the launcher due to the fact that they send aSIGHUGsignal and the JVM handles the signal and stops the still running (waiting) launcher-patcher. This required users to download the launcher themselves when we did changes to the launcher.Modification
Introduced a handler for
SIGHUPin the launcher-patcher that ignores the signal. Furthermore, this introduces a 5 second timeout when waiting for the launcher to exit before running the patcher as the launcher should not take that long to stop and to prevent replacing the next running launcher instance getting replaced while running.Result
Launcher updates are properly working when using
screenortmux