Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add System.exit #13510

wants to merge 1 commit into from

Conversation

koppor
Copy link
Member

@koppor koppor commented Jul 8, 2025

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

  • I own the copyright of the code submitted and I license it under the MIT license
  • [/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • [/] Tests created for changes (if applicable)
  • [/] Manually tested changed features in running JabRef (always required)
  • [/] Screenshots added in PR description (if change is visible to the user)
  • [/] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 8, 2025
@@ -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);
Copy link

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.

Comment on lines +509 to +510
// Just to be sure that we do not leave any threads running
System.exit(0);
Copy link

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.

@Siedlerchr
Copy link
Member

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?
and you are sure it's not gradle?

@koppor
Copy link
Member Author

koppor commented Jul 9, 2025

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? and you are sure it's not gradle?

Yes, therefore, I wrote following - see the yellow-marked text --> first sentence in the PR description.

image

@koppor
Copy link
Member Author

koppor commented Jul 11, 2025

On current main (with some tests in CAYW), grizzly is shut down, but something else is not:

image

Remember: One can start Visual VM by ./gg.cmd gh/oracle/visualvm - gg.cmd is available at https://github.com/eirikb/gg)

@koppor
Copy link
Member Author

koppor commented Jul 15, 2025

We log to JavaFX - maybe, this is causing troubles:

2025-07-15 21:03:58 [] org.glassfish.grizzly.http.server.NetworkListener.shutdownNow()
INFO: Stopped listener bound to [127.0.0.1:23119]
2025-07-15 21:04:28 [] org.jabref.gui.util.UiThreadList.sourceChanged()
ERROR: Error while running on JavaFX thread: java.lang.InterruptedException
        at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1139)
        at java.base/java.util.concurrent.CountDownLatch.await(CountDownLatch.java:230)
        at org.jabref/org.jabref.gui.util.UiThreadList.sourceChanged(UiThreadList.java:32)
        at javafx.base@24.0.1/javafx.collections.transformation.TransformationList.lambda$getListener$0(TransformationList.java:105)
        at javafx.base@24.0.1/javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
        at javafx.base@24.0.1/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
        at javafx.base@24.0.1/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
        at javafx.base@24.0.1/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
        at javafx.base@24.0.1/javafx.collections.FXCollections$UnmodifiableObservableListImpl.lambda$new$0(FXCollections.java:947)
        at javafx.base@24.0.1/javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
        at javafx.base@24.0.1/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
        at javafx.base@24.0.1/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
        at javafx.base@24.0.1/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
        at javafx.base@24.0.1/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
        at javafx.base@24.0.1/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
        at javafx.base@24.0.1/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
        at javafx.base@24.0.1/javafx.collections.ModifiableObservableListBase.add(ModifiableObservableListBase.java:200)
        at java.base/java.util.AbstractList.add(AbstractList.java:113)
        at org.jabref/org.jabref.gui.logging.LogMessages.add(LogMessages.java:30)
        at org.jabref/org.jabref.gui.logging.GuiWriter.write(GuiWriter.java:28)
        at org.tinylog.impl@2.7.0/org.tinylog.core.TinylogLoggingProvider.output(TinylogLoggingProvider.java:259)
        at org.tinylog.impl@2.7.0/org.tinylog.core.TinylogLoggingProvider.log(TinylogLoggingProvider.java:186)
        at org.tinylog.api.slf4j@2.7.0/org.tinylog.slf4j.AbstractTinylogLogger.log(AbstractTinylogLogger.java:501)
        at jul.to.slf4j@2.0.17/org.slf4j.bridge.SLF4JBridgeHandler.callLocationAwareLogger(SLF4JBridgeHandler.java:232)
        at jul.to.slf4j@2.0.17/org.slf4j.bridge.SLF4JBridgeHandler.publish(SLF4JBridgeHandler.java:313)
        at java.logging/java.util.logging.Logger.log(Logger.java:962)
        at java.logging/java.util.logging.Logger.doLog(Logger.java:989)
        at java.logging/java.util.logging.Logger.log(Logger.java:1055)
        at org.glassfish.grizzly.http.server@4.0.2/org.glassfish.grizzly.http.server.NetworkListener.shutdownNow(NetworkListener.java:859)
        at org.glassfish.grizzly.http.server@4.0.2/org.glassfish.grizzly.http.server.HttpServer.removeListener(HttpServer.java:188)
        at org.glassfish.grizzly.http.server@4.0.2/org.glassfish.grizzly.http.server.HttpServer.shutdownNow(HttpServer.java:382)
        at org.jabref.jabsrv/org.jabref.http.manager.HttpServerThread.interrupt(HttpServerThread.java:50)
        at org.jabref.jabsrv/org.jabref.http.manager.HttpServerManager.stop(HttpServerManager.java:41)
        at org.jabref/org.jabref.gui.JabRefGUI.lambda$stop$4(JabRefGUI.java:458)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:545)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:328)
        at java.base/java.lang.VirtualThread.run(VirtualThread.java:466)
2025-07-15 21:04:58 [JavaFX Application Thread] org.jabref.logic.util.HeadlessExecutorService.gracefullyShutdown()
ERROR: HeadlessExecutorService did not terminate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants