-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add System.exit #13510
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
base: main
Are you sure you want to change the base?
Add System.exit #13510
Conversation
@@ -505,5 +505,8 @@ public void stop() { | |||
} | |||
|
|||
LOGGER.trace("Finished stop"); | |||
|
|||
// Just to be sure that we do not leave any threads running | |||
System.exit(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using System.exit() is a harsh way to terminate an application and can prevent proper cleanup. JavaFX's Platform.exit() should handle the shutdown gracefully.
// Just to be sure that we do not leave any threads running | ||
System.exit(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment merely restates what the code does without providing additional context or reasoning for this drastic measure, violating the principle of meaningful comments.
I don't get the problem. We already have shutdown cleanup and stopping of threads. Have you checked with Visual VM what's going on? |
On current main (with some tests in CAYW), grizzly is shut down, but something else is not: ![]()
|
We log to JavaFX - maybe, this is causing troubles:
|
My JabRef had the grizzly threads running on exit. I don't have the time to debug now.
Maybe, in future, there will be other things taking time to properly shut down.
Therfore, I propose to "just" end the JVM so that nothing is left behind by JabRef.
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)