Exception after cleanup entries dialog with rename PDF option enabled

[sfo]

This is similar to issue #4817, which has been closed already. However, the exception is different.

Version:

JabRef 5.0-dev
Linux 4.4.0-154-generic amd64
Java 1.8.0_212

started via ./gradlew run at commit add35be

Steps to reproduce:

1. attach a file to an entry (filename must have different naming scheme than the format defined for cleanup)
2. select and cleanup the entry with the "Rename PDFs to given filename format pattern" option enabled
3. click OK to start cleanup process

Besides the exception, the renaming works, nevertheless.

Error Log:

Detail information:

Doing a cleanup for 1 entries...
Enabling live reloading of /home/stanley/Development/stuff/jabref/build/resources/main/org/jabref/gui/Base.css
Uncaught exception occurred in Thread[pool-3-thread-5,5,main]
java.lang.IllegalStateException: Not on FX application thread; currentThread = pool-3-thread-5
	at com.sun.javafx.tk.Toolkit.checkFxUserThread(Toolkit.java:229)
	at com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(QuantumToolkit.java:423)
	at javafx.scene.Parent$2.onProposedChange(Parent.java:367)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:113)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:108)
	at com.sun.javafx.scene.control.skin.LabeledSkinBase.updateChildren(LabeledSkinBase.java:575)
	at com.sun.javafx.scene.control.skin.LabeledSkinBase.handleControlPropertyChanged(LabeledSkinBase.java:181)
	at com.sun.javafx.scene.control.skin.ListCellSkin.handleControlPropertyChanged(ListCellSkin.java:49)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase.lambda$registerChangeListener$144(BehaviorSkinBase.java:197)
	at com.sun.javafx.scene.control.MultiplePropertyChangeListenerHandler$1.changed(MultiplePropertyChangeListenerHandler.java:55)
	at javafx.beans.value.WeakChangeListener.changed(WeakChangeListener.java:89)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.ObjectPropertyBase.fireValueChangedEvent(ObjectPropertyBase.java:105)
	at javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:146)
	at javafx.css.StyleableObjectProperty.set(StyleableObjectProperty.java:82)
	at javafx.beans.property.ObjectProperty.setValue(ObjectProperty.java:69)
	at javafx.scene.control.Labeled.setGraphic(Labeled.java:423)
	at org.jabref.gui.util.ViewModelListCellFactory$1.updateItem(ViewModelListCellFactory.java:149)
	at javafx.scene.control.ListCell.updateItem(ListCell.java:480)
	at javafx.scene.control.ListCell.lambda$new$77(ListCell.java:167)
	at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
	at com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at com.sun.javafx.collections.ObservableListWrapper.remove(ObservableListWrapper.java:167)
	at com.sun.javafx.binding.BidirectionalContentBinding$ListContentBinding.onChanged(BidirectionalContentBinding.java:135)
	at com.sun.javafx.binding.ListExpressionHelper$Generic.notifyListeners(ListExpressionHelper.java:593)
	at com.sun.javafx.binding.ListExpressionHelper$Generic.fireValueChangedEvent(ListExpressionHelper.java:571)
	at com.sun.javafx.binding.ListExpressionHelper.fireValueChangedEvent(ListExpressionHelper.java:109)
	at javafx.beans.property.ListPropertyBase.fireValueChangedEvent(ListPropertyBase.java:200)
	at javafx.beans.property.ListPropertyBase.lambda$new$22(ListPropertyBase.java:56)
	at com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:164)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at javafx.collections.ModifiableObservableListBase.setAll(ModifiableObservableListBase.java:90)
	at javafx.beans.binding.ListExpression.setAll(ListExpression.java:370)
	at org.jabref.gui.util.BindingsHelper.lambda$bindContentBidirectional$652(BindingsHelper.java:125)
	at org.jabref.gui.util.BindingsHelper$BidirectionalListBinding.changed(BindingsHelper.java:262)
	at com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:361)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.StringPropertyBase.fireValueChangedEvent(StringPropertyBase.java:103)
	at javafx.beans.property.StringPropertyBase.markInvalid(StringPropertyBase.java:110)
	at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:144)
	at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:49)
	at javafx.beans.property.StringProperty.setValue(StringProperty.java:65)
	at javafx.beans.property.StringProperty.setValue(StringProperty.java:57)
	at org.jabref.gui.util.BindingsHelper$BidirectionalBinding.updateLocked(BindingsHelper.java:234)
	at org.jabref.gui.util.BindingsHelper$BidirectionalBinding.changedB(BindingsHelper.java:227)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.binding.ObjectBinding.invalidate(ObjectBinding.java:172)
	at com.sun.javafx.binding.BindingHelperObserver.invalidated(BindingHelperObserver.java:51)
	at com.sun.javafx.collections.MapListenerHelper$Generic.fireValueChangedEvent(MapListenerHelper.java:320)
	at com.sun.javafx.collections.MapListenerHelper.fireValueChangedEvent(MapListenerHelper.java:72)
	at com.sun.javafx.collections.ObservableMapWrapper.callObservers(ObservableMapWrapper.java:115)
	at com.sun.javafx.collections.ObservableMapWrapper.put(ObservableMapWrapper.java:169)
	at org.jabref.model.entry.BibEntry.setField(BibEntry.java:413)
	at org.jabref.model.entry.BibEntry.setField(BibEntry.java:439)
	at org.jabref.model.entry.BibEntry.setFiles(BibEntry.java:801)
	at org.jabref.logic.cleanup.RenamePdfCleanup.cleanup(RenamePdfCleanup.java:57)
	at org.jabref.logic.cleanup.CleanupWorker.cleanup(CleanupWorker.java:31)
	at org.jabref.gui.cleanup.CleanupAction.doCleanup(CleanupAction.java:90)
	at org.jabref.gui.cleanup.CleanupAction.cleanup(CleanupAction.java:133)
	at org.jabref.gui.cleanup.CleanupAction.lambda$action$810(CleanupAction.java:65)
	at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:55)
	at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:52)
	at org.jabref.gui.util.DefaultTaskExecutor$1.call(DefaultTaskExecutor.java:122)
	at javafx.concurrent.Task$TaskCallable.call(Task.java:1423)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Uncaught exception occurred in Thread[pool-3-thread-5,5,main]
java.lang.IllegalStateException: Not on FX application thread; currentThread = pool-3-thread-5
	at com.sun.javafx.tk.Toolkit.checkFxUserThread(Toolkit.java:229)
	at com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(QuantumToolkit.java:423)
	at javafx.scene.Parent$2.onProposedChange(Parent.java:367)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:113)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:108)
	at com.sun.javafx.scene.control.skin.LabeledSkinBase.updateChildren(LabeledSkinBase.java:580)
	at com.sun.javafx.scene.control.skin.LabeledSkinBase.handleControlPropertyChanged(LabeledSkinBase.java:181)
	at com.sun.javafx.scene.control.skin.ListCellSkin.handleControlPropertyChanged(ListCellSkin.java:49)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase.lambda$registerChangeListener$144(BehaviorSkinBase.java:197)
	at com.sun.javafx.scene.control.MultiplePropertyChangeListenerHandler$1.changed(MultiplePropertyChangeListenerHandler.java:55)
	at javafx.beans.value.WeakChangeListener.changed(WeakChangeListener.java:89)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.ObjectPropertyBase.fireValueChangedEvent(ObjectPropertyBase.java:105)
	at javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:146)
	at javafx.css.StyleableObjectProperty.set(StyleableObjectProperty.java:82)
	at javafx.beans.property.ObjectProperty.setValue(ObjectProperty.java:69)
	at javafx.scene.control.Labeled.setGraphic(Labeled.java:423)
	at org.jabref.gui.util.ViewModelListCellFactory$1.updateItem(ViewModelListCellFactory.java:157)
	at javafx.scene.control.ListCell.updateItem(ListCell.java:471)
	at javafx.scene.control.ListCell.lambda$new$77(ListCell.java:167)
	at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
	at com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at javafx.collections.ModifiableObservableListBase.addAll(ModifiableObservableListBase.java:113)
	at com.sun.javafx.binding.BidirectionalContentBinding$ListContentBinding.onChanged(BidirectionalContentBinding.java:138)
	at com.sun.javafx.binding.ListExpressionHelper$Generic.notifyListeners(ListExpressionHelper.java:593)
	at com.sun.javafx.binding.ListExpressionHelper$Generic.fireValueChangedEvent(ListExpressionHelper.java:571)
	at com.sun.javafx.binding.ListExpressionHelper.fireValueChangedEvent(ListExpressionHelper.java:109)
	at javafx.beans.property.ListPropertyBase.fireValueChangedEvent(ListPropertyBase.java:200)
	at javafx.beans.property.ListPropertyBase.lambda$new$22(ListPropertyBase.java:56)
	at com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:164)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at javafx.collections.ModifiableObservableListBase.setAll(ModifiableObservableListBase.java:90)
	at javafx.beans.binding.ListExpression.setAll(ListExpression.java:370)
	at org.jabref.gui.util.BindingsHelper.lambda$bindContentBidirectional$652(BindingsHelper.java:125)
	at org.jabref.gui.util.BindingsHelper$BidirectionalListBinding.changed(BindingsHelper.java:262)
	at com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:361)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.StringPropertyBase.fireValueChangedEvent(StringPropertyBase.java:103)
	at javafx.beans.property.StringPropertyBase.markInvalid(StringPropertyBase.java:110)
	at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:144)
	at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:49)
	at javafx.beans.property.StringProperty.setValue(StringProperty.java:65)
	at javafx.beans.property.StringProperty.setValue(StringProperty.java:57)
	at org.jabref.gui.util.BindingsHelper$BidirectionalBinding.updateLocked(BindingsHelper.java:234)
	at org.jabref.gui.util.BindingsHelper$BidirectionalBinding.changedB(BindingsHelper.java:227)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.binding.ObjectBinding.invalidate(ObjectBinding.java:172)
	at com.sun.javafx.binding.BindingHelperObserver.invalidated(BindingHelperObserver.java:51)
	at com.sun.javafx.collections.MapListenerHelper$Generic.fireValueChangedEvent(MapListenerHelper.java:320)
	at com.sun.javafx.collections.MapListenerHelper.fireValueChangedEvent(MapListenerHelper.java:72)
	at com.sun.javafx.collections.ObservableMapWrapper.callObservers(ObservableMapWrapper.java:115)
	at com.sun.javafx.collections.ObservableMapWrapper.put(ObservableMapWrapper.java:169)
	at org.jabref.model.entry.BibEntry.setField(BibEntry.java:413)
	at org.jabref.model.entry.BibEntry.setField(BibEntry.java:439)
	at org.jabref.model.entry.BibEntry.setFiles(BibEntry.java:801)
	at org.jabref.logic.cleanup.RenamePdfCleanup.cleanup(RenamePdfCleanup.java:57)
	at org.jabref.logic.cleanup.CleanupWorker.cleanup(CleanupWorker.java:31)
	at org.jabref.gui.cleanup.CleanupAction.doCleanup(CleanupAction.java:90)
	at org.jabref.gui.cleanup.CleanupAction.cleanup(CleanupAction.java:133)
	at org.jabref.gui.cleanup.CleanupAction.lambda$action$810(CleanupAction.java:65)
	at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:55)
	at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:52)
	at org.jabref.gui.util.DefaultTaskExecutor$1.call(DefaultTaskExecutor.java:122)
	at javafx.concurrent.Task$TaskCallable.call(Task.java:1423)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

One entry needed a clean up
Enabling live reloading of /home/stanley/Development/stuff/jabref/build/resources/main/org/jabref/gui/Base.css

[r0light]

The reason for this seems to be the Binding created in the constructor of LinkedFilesEditorViewModel:

BindingsHelper.bindContentBidirectional(
                                                files,
                                                text,
                                                LinkedFilesEditorViewModel::getStringRepresentation,
                                                this::parseToFileViewModel);

For this Binding, the fix from #4871 (and #4964 ) does not seem to work, because the binding is triggered directly from the call entry.setFiles(files) in RenamePdfCleanup which is executed in a background thread and therefore leads to the Exception.
Although it is a bit strange, because for other operations like DoiCleanup it is working fine and the method UiThreadList.sourceChanged is called (i had a look at it to compare the behaviour):

"pool-3-thread-3@7731" prio=5 tid=0x21 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at org.jabref.gui.util.UiThreadList.sourceChanged(UiThreadList.java:15)
	  at javafx.collections.transformation.TransformationList.lambda$getListener$0(TransformationList.java:106)
	  at javafx.collections.transformation.TransformationList$$Lambda$424.901219265.onChanged(Unknown Source:-1)
	  at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
	  at com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:164)
	  at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	  at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	  at javafx.collections.FXCollections$UnmodifiableObservableListImpl.lambda$new$0(FXCollections.java:955)
	  at javafx.collections.FXCollections$UnmodifiableObservableListImpl$$Lambda$198.652863251.onChanged(Unknown Source:-1)
	  at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
	  at com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329)
	  at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	  at javafx.collections.FXCollections$SynchronizedObservableList.lambda$new$0(FXCollections.java:1204)
	  at javafx.collections.FXCollections$SynchronizedObservableList$$Lambda$296.756699190.onChanged(Unknown Source:-1)
	  at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
	  at com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:164)
	  at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	  at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	  at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:485)
	  at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	  at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	  at com.sun.javafx.collections.ObservableListWrapper.access$200(ObservableListWrapper.java:45)
	  at com.sun.javafx.collections.ObservableListWrapper$1$1.invalidated(ObservableListWrapper.java:75)
	  at com.sun.javafx.collections.MapListenerHelper$Generic.fireValueChangedEvent(MapListenerHelper.java:320)
	  at com.sun.javafx.collections.MapListenerHelper.fireValueChangedEvent(MapListenerHelper.java:72)
	  at com.sun.javafx.collections.ObservableMapWrapper.callObservers(ObservableMapWrapper.java:115)
	  at com.sun.javafx.collections.ObservableMapWrapper.put(ObservableMapWrapper.java:169)
	  at org.jabref.model.entry.BibEntry.setField(BibEntry.java:414)
	  at org.jabref.model.entry.BibEntry.setField(BibEntry.java:440)
	  at org.jabref.logic.cleanup.DoiCleanup.cleanup(DoiCleanup.java:42)
	  at org.jabref.logic.cleanup.CleanupWorker.cleanup(CleanupWorker.java:31)
	  at org.jabref.gui.cleanup.CleanupAction.doCleanup(CleanupAction.java:90)
	  at org.jabref.gui.cleanup.CleanupAction.cleanup(CleanupAction.java:133)
	  at org.jabref.gui.cleanup.CleanupAction.lambda$action$1(CleanupAction.java:65)
	  at org.jabref.gui.cleanup.CleanupAction$$Lambda$1978.649778965.run(Unknown Source:-1)
	  at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:55)
	  at org.jabref.gui.util.BackgroundTask$2.call(BackgroundTask.java:52)
	  at org.jabref.gui.util.DefaultTaskExecutor$1.call(DefaultTaskExecutor.java:122)
	  at javafx.concurrent.Task$TaskCallable.call(Task.java:1425)
	  at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	  at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	  at java.lang.Thread.run(Thread.java:834)

@tobiasdiez What about adding a Platform.runLater call in the class BindingsHelper.BidirectionalMapBinding (which is used for BindingsHelper.bindContentBidirectional) to ensure all updates are executed on the JavaFX Thread?

[Siedlerchr]

See also #5010

[tobiasdiez]

@r0light Thanks for investigating this issue. I agree that adding runLater would fix the problem. However, this also has some negative consequences because these observable lists are also used in non-ui situations (for example unit tests). For this reason I would prefer a wrapper around the original list that pushes the changes to the ui thread (and then binding this wrapper to the control).

I had a quick look and found griffon-javafx which includes such wrappers: http://griffon-framework.org/guide/2.11.0/api/griffon/javafx/beans/binding/UIThreadAwareBindings.html
We can simply use this library (or reimplement the small subset of the wrappers that we need at the moment starting from their implementation). @r0light are you willing to work on this?

[r0light]

@tobiasdiez yes i would like to have a look at this.
I created a pull request #5289 where i started working on it. Is that what you had in mind?