ExpressionHelper thread-safety

[Christopher Schnick]

Hello,

I encountered a rare exception where adding listeners to an observable 
value might break when they are added concurrently. This is due to 
ExpressionHelper not being synchronized. I thought about how to fix this 
on my side, but it is very difficult to do. As this is not a typical 
platform thread issue, in my opinion it should be possible to add 
listeners to one observable value from any thread without having to 
think about any potential synchronization issues (which I can't solve 
other than just running everything on one thread).

Even worse, due to the size and array being two different variables and 
being incremented unsafely, once such a concurrent modification occurs, 
this invalid state will persist permanently and will cause exceptions on 
any further method call as well. The only solution is to restart the 
application.

This is how a stack trace looks like when this occurs:

21:25:38:840 - error: Index 2 out of bounds for length 2
java.lang.ArrayIndexOutOfBoundsException: Index 2 out of bounds for length 2
     at 
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:248)
     at 
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:200)
     at 
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:65)
     at 
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:86)
     at javafx.beans.binding.StringBinding.bind(StringBinding.java:114)
     at javafx.beans.binding.Bindings$7.<init>(Bindings.java:428)
     at javafx.beans.binding.Bindings.createStringBinding(Bindings.java:426)
     at 
io.xpipe.app.util.StoreStateFormat.shellEnvironment(StoreStateFormat.java:24)
     at 
io.xpipe.ext.proc.env.ShellEnvironmentStoreProvider.informationString(ShellEnvironmentStoreProvider.java:155)
     at 
io.xpipe.app.comp.store.StoreEntryWrapper.update(StoreEntryWrapper.java:228)
     at 
io.xpipe.app.comp.store.StoreViewState.lambda$updateContent$1(StoreViewState.java:147)
     at java.lang.Iterable.forEach(Iterable.java:75)
     at 
io.xpipe.app.comp.store.StoreViewState.updateContent(StoreViewState.java:147)
     at io.xpipe.app.comp.store.StoreViewState.init(StoreViewState.java:93)
     at 
io.xpipe.app.core.mode.BaseMode.lambda$onSwitchTo$1(BaseMode.java:109)
     at io.xpipe.app.util.ThreadHelper.lambda$load$0(ThreadHelper.java:78)
     at java.lang.Thread.run(Thread.java:1447)

21:25:38:847 - error: Index 3 out of bounds for length 2
java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 2
     at 
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:248)
     at 
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:200)
     at 
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:65)
     at 
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:86)
     at javafx.beans.binding.StringBinding.bind(StringBinding.java:114)
     at javafx.beans.binding.Bindings$7.<init>(Bindings.java:428)
     at javafx.beans.binding.Bindings.createStringBinding(Bindings.java:426)
     at 
io.xpipe.app.util.StoreStateFormat.shellEnvironment(StoreStateFormat.java:24)
     at 
io.xpipe.ext.proc.env.ShellEnvironmentStoreProvider.informationString(ShellEnvironmentStoreProvider.java:155)
     at 
io.xpipe.app.comp.store.StoreEntryWrapper.update(StoreEntryWrapper.java:228)
     at 
io.xpipe.app.comp.store.StoreEntryWrapper.lambda$setupListeners$3(StoreEntryWrapper.java:143)
     at 
io.xpipe.app.util.PlatformThread.lambda$runLaterIfNeeded$0(PlatformThread.java:318)
     at 
com.sun.javafx.application.PlatformImpl.lambda$runLater$4(PlatformImpl.java:424)
     at 
com.sun.glass.ui.InvokeLaterDispatcher$Future.run$$$capture(InvokeLaterDispatcher.java:95)
     at 
com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java)

This full log goes up to index 50 out of bounds due to the recurring 
nature of this exception.

Looking at the implementation of ExpressionHelper, I don't see any harm 
in just synchronizing the methods, at least from my perspective. But I 
guess that is up to the developers to decide. The only real solution I 
have as an application developer is to perform all initialization on one 
thread or just hope that this error is rare enough, both of which aren't 
great options. So I hope that a potential synchronization of the 
ExpressionHelper methods can be considered.

Best
Christopher Schnick