On 23/04/2025 20:59, Andy Goryachev wrote: > > Even though JavaFX explicitly permits creating Nodes and Scenes in a > thread other than the Application Thread, I think it is still a bad > idea, and I would strongly suggest against doing so. The code might > work - initially - but you will soon discover that it presents a > constant source of issues, especially after the application is deployed. > > > > I would also question the value of such a design. How many > milliseconds is being saved by trying to instantiate Nodes in a > background thread? If you create only a few objects, there is > absolutely no benefit (and a huge maintenance burden), but if there > are too many objects created then maybe one is doing something wrong, > perhaps instead one should try to create things in batches? > The question isn't so much how much milliseconds it saves; the question is, do you want to burden the FX thread with any multi-millisecond action, including construction of a big component (like a new Tab full of controls, or an entire Window)? The answer to that should be a clear no; anything that takes more than few milliseconds will mean stuttering animations as frames will get dropped.
Constructing components in a background thread is perfectly fine; you however should NOT connect these components on the background thread to any properties that may receive FX thread callbacks. So the proper way to construct a UI component in the background is: - Construct large graph on a background thread, but do not connect to any external/global properties yet; take as much time as you need, the rest of the UI will keep responding and animating - Then after the heavy work is done, connect to any external properties on the FX thread - When the UI has served its purpose, don't forget to disconnect from any external properties This can be largely transparent by using the "when" construct; using this construct you can make links with the external properties immediately even on the background thread. With the proper when-condition, the actual listeners are added just-in-time, and on the correct thread (typically, you'd use a condition that tracks whether the control is part of an active scene graph, like node->scene->window->isShowing). --John > > > So my recommendation would remain the same: please don't. Always > access JavaFX objects from the Application Thread. > > > > -andy > > > > > > > > > > *From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of Kevin > Rushforth <kevin.rushfo...@oracle.com> > *Date: *Wednesday, April 23, 2025 at 11:41 > *To: *openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> > *Subject: *Re: ExpressionHelper thread-safety > > This came up most recently in the discussion of > https://github.com/openjdk/jfx/pull/1697 > > As noted by you and in that PR, properties are not thread-safe. If two > threads add a listener concurrently, or if one thread adds a listener > while and another thread notifies the listeners, it is likely to fail. > > So the question is: Is it worth doing something about this? And if so, > how far do we go? > > Making the add/remove listeners operations on ExpressionHelper (and > related classes?) thread-safe so that listeners could be added or > removed on any thread concurrently with each other and with the > operation off firing a listener probably wouldn't be too hard or have > much downside (the performance impact should be negligible and it is > unlikely to cause a deadlock). > > You still wouldn't be able to modify a property on more than one thread, > nor control the thread on which listeners are notified (they are > notified on the thread that mutates the property), so it won't magically > solve all your threading issues; and you still would need to deal with > the fact that your listener can be called on a different thread than the > one which added it. > > I'd like to hear from Andy, John, and others as to whether they think > there is value in providing partial thread-safety for the add/remove > listener methods of properties. > > -- Kevin > > > On 4/23/2025 9:58 AM, Christopher Schnick wrote: > > 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 > > >