Skip to content

Conversation

@koppor
Copy link
Member

@koppor koppor commented Aug 4, 2024

JabRef collected all run tasks. This was confusing. This PR fixes this.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

@calixtus calixtus added component: ui status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Aug 4, 2024
@calixtus calixtus enabled auto-merge August 4, 2024 15:23
@calixtus calixtus added this pull request to the merge queue Aug 4, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

Merged via the queue into main with commit 2c8fc01 Aug 4, 2024
@calixtus calixtus deleted the demo-background branch August 4, 2024 15:46
@koppor
Copy link
Member Author

koppor commented Aug 4, 2024

Follow-up depends on controlsfx/controlsfx#1559

@LoayGhreeb
Copy link
Member

@koppor, @calixtus
This PR causes an exception that prevents JabRef from shutting down properly. When I close JabRef while a background task is running, Will asked either to wait for the task to finish or cancel the task and shutdown. If I choose to cancel, an exception is thrown, JabRef window closes, but IntelliJ shows that JabRef is still running.

Exception
Caused by: java.util.NoSuchElementException: java.lang.IndexOutOfBoundsException
	at java.base/java.util.AbstractList$Itr.next(AbstractList.java:379)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
	at [email protected]/org.jabref.gui.util.UiTaskExecutor.shutdown(UiTaskExecutor.java:131)
	at [email protected]/org.jabref.gui.JabRefGUI.shutdownThreadPools(JabRefGUI.java:376)
	at [email protected]/org.jabref.gui.JabRefGUI.stop(JabRefGUI.java:366)
	at [email protected]/com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$10(LauncherImpl.java:858)
	at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runAndWait$12(PlatformImpl.java:483)
	at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
	at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at [email protected]/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at [email protected]/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:184)
	... 1 more
Caused by: java.lang.IndexOutOfBoundsException
	at [email protected]/javafx.collections.transformation.FilteredList.get(FilteredList.java:169)
	at [email protected]/com.tobiasdiez.easybind.MappedList.get(MappedList.java:31)
	at java.base/java.util.AbstractList$Itr.next(AbstractList.java:373)
	... 15 more

I tried to revert this commit, and the issue was resolved.

To reproduce the issue:

  • Checkout the Lucene branch (Lucene search #11542).
  • Open a library with PDFs.
  • If the files are already indexed, use "Rebuild Index."
  • Try to close JabRef before indexing is finished.
  • Click Yes when prompted. image

StateManager stateManager = Injector.instantiateModelOrService(StateManager.class);
if (stateManager != null) {
stateManager.getBackgroundTasks().stream().filter(task -> !task.isDone()).forEach(Task::cancel);
stateManager.getRunningBackgroundTasks().stream().forEach(Task::cancel);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was that canceling a task removes items from the runningBackgroundTasks list since it's a filtered list. Removing items during iteration causes an IndexOutOfBoundsException.

Reverting this line will fix the issue.
I will reopen the PR.

@LoayGhreeb LoayGhreeb restored the demo-background branch September 24, 2024 05:56
@LoayGhreeb LoayGhreeb mentioned this pull request Sep 24, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: ui 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.

4 participants