fix: jvm hangs when System.exit or bugsnag.close is not called #157
Conversation
This file contains 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
tomlongridge
force-pushed
the
tom/fix-jvm-shutdown
branch
3 times, most recently
from
8e60dd1 to
01c6ea6
Compare
fractalwrench
approved these changes
Nov 9, 2020
| * This is to prevent applications from hanging waiting for the sessions scheduled task | ||
| * because daemon threads will be terminated on application shutdown | ||
| */ | ||
| public class DaemonThreadFactory implements ThreadFactory { |
There was a problem hiding this comment.
The original PR had a test for this class which may be worth reinstating: https://github.com/bugsnag/bugsnag-java/pull/143/files
tomlongridge
force-pushed
the
tom/fix-jvm-shutdown
branch
from
01c6ea6 to
47e71fa
Compare
The fix to prevent a resource leak in #143 removed the task scheduler running as a daemon thread and so doesn't exit unless the application is explicitly exited (via System.exit) or the thread is explicitly stopped (via bugsnag.close). This fix should be the benefits of both #143 and #121 to gracefully shutdown in both use cases. Updated the Spring Boot app test and example to remove explict System.exit call as this should not be required. fixes #151
tomlongridge
force-pushed
the
tom/fix-jvm-shutdown
branch
from
47e71fa to
3907c29
Compare
Merged
|
Nice 👏 |
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.
Goal
The fix to prevent a resource leak in #143 improved the shutdown hook to prevent the resource leak (calling
ExceptionHandler.disableandconfig.delivery.close) but also reverted the task scheduler from running as a daemon thread. Therefore, in a console app, or similar, which doesn't explicitly callSystem.exit, the JVM waits indefinitely for thesessionExecutorServiceto stop before triggering the shutdown hook.This PR should be the benefits of both #143 and #121 to gracefully shutdown in both use cases.
Fixes #151
Changeset
DaemonThreadFactory(removed in Remove use of daemon threads, fixing potential resource leak #143)System.exitcalls from the Spring Boot example as the explicit call should not be required.Testing
System.exitcall to test this in future.