<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
On 23/04/2025 20:59, Andy Goryachev wrote:<br>
<blockquote type="cite"
cite="mid:BYAPR10MB30131847B13AD54A4257017CE5BA2@BYAPR10MB3013.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="Generator"
content="Microsoft Word 15 (filtered medium)">
<style>@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face
{font-family:Aptos;
panose-1:2 11 0 4 2 2 2 2 2 4;}@font-face
{font-family:"Iosevka Fixed SS16";
panose-1:2 0 5 9 3 0 0 0 0 4;}@font-face
{font-family:"Times New Roman \(Body CS\)";
panose-1:2 11 6 4 2 2 2 2 2 4;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:10.0pt;
font-family:"Aptos",sans-serif;}a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Iosevka Fixed SS16";
color:windowtext;}.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}div.WordSection1
{page:WordSection1;}</style>
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">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?</span></p>
</div>
</blockquote>
<p>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. <br>
</p>
<p>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:</p>
<p>- 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<br>
- Then after the heavy work is done, connect to any external
properties on the FX thread<br>
- When the UI has served its purpose, don't forget to disconnect
from any external properties<br>
<br>
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).</p>
<p>--John<br>
</p>
<blockquote type="cite"
cite="mid:BYAPR10MB30131847B13AD54A4257017CE5BA2@BYAPR10MB3013.namprd10.prod.outlook.com">
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">So
my recommendation would remain the same: please don't.
Always access JavaFX objects from the Application Thread.<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">-andy<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<div id="mail-editor-reference-message-container">
<div>
<div>
<div
style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span
style="font-size:12.0pt;color:black">From:
</span></b><span
style="font-size:12.0pt;color:black">openjfx-dev
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev-retn@openjdk.org"><openjfx-dev-retn@openjdk.org></a> on behalf of
Kevin Rushforth <a class="moz-txt-link-rfc2396E" href="mailto:kevin.rushforth@oracle.com"><kevin.rushforth@oracle.com></a><br>
<b>Date: </b>Wednesday, April 23, 2025 at 11:41<br>
<b>To: </b><a class="moz-txt-link-abbreviated" href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev@openjdk.org"><openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: ExpressionHelper thread-safety<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span
style="font-size:11.0pt">This came up most recently
in the discussion of
<br>
<a href="https://github.com/openjdk/jfx/pull/1697"
moz-do-not-send="true"
class="moz-txt-link-freetext">https://github.com/openjdk/jfx/pull/1697</a><br>
<br>
As noted by you and in that PR, properties are not
thread-safe. If two <br>
threads add a listener concurrently, or if one
thread adds a listener <br>
while and another thread notifies the listeners, it
is likely to fail.<br>
<br>
So the question is: Is it worth doing something
about this? And if so, <br>
how far do we go?<br>
<br>
Making the add/remove listeners operations on
ExpressionHelper (and <br>
related classes?) thread-safe so that listeners
could be added or <br>
removed on any thread concurrently with each other
and with the <br>
operation off firing a listener probably wouldn't be
too hard or have <br>
much downside (the performance impact should be
negligible and it is <br>
unlikely to cause a deadlock).<br>
<br>
You still wouldn't be able to modify a property on
more than one thread, <br>
nor control the thread on which listeners are
notified (they are <br>
notified on the thread that mutates the property),
so it won't magically <br>
solve all your threading issues; and you still would
need to deal with <br>
the fact that your listener can be called on a
different thread than the <br>
one which added it.<br>
<br>
I'd like to hear from Andy, John, and others as to
whether they think <br>
there is value in providing partial thread-safety
for the add/remove <br>
listener methods of properties.<br>
<br>
-- Kevin<br>
<br>
<br>
On 4/23/2025 9:58 AM, Christopher Schnick wrote:<br>
> Hello,<br>
><br>
> I encountered a rare exception where adding
listeners to an observable <br>
> value might break when they are added
concurrently. This is due to <br>
> ExpressionHelper not being synchronized. I
thought about how to fix <br>
> this on my side, but it is very difficult to
do. As this is not a <br>
> typical platform thread issue, in my opinion it
should be possible to <br>
> add listeners to one observable value from any
thread without having <br>
> to think about any potential synchronization
issues (which I can't <br>
> solve other than just running everything on one
thread).<br>
><br>
> Even worse, due to the size and array being two
different variables <br>
> and being incremented unsafely, once such a
concurrent modification <br>
> occurs, this invalid state will persist
permanently and will cause <br>
> exceptions on any further method call as well.
The only solution is to <br>
> restart the application.<br>
><br>
> This is how a stack trace looks like when this
occurs:<br>
><br>
> 21:25:38:840 - error: Index 2 out of bounds for
length 2<br>
> java.lang.ArrayIndexOutOfBoundsException: Index
2 out of bounds for <br>
> length 2<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:248)<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:200)<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:65)<br>
> at <br>
>
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:86)<br>
> at
javafx.beans.binding.StringBinding.bind(StringBinding.java:114)<br>
> at
javafx.beans.binding.Bindings$7.<init>(Bindings.java:428)<br>
> at <br>
>
javafx.beans.binding.Bindings.createStringBinding(Bindings.java:426)<br>
> at <br>
>
io.xpipe.app.util.StoreStateFormat.shellEnvironment(StoreStateFormat.java:24)<br>
> at <br>
>
io.xpipe.ext.proc.env.ShellEnvironmentStoreProvider.informationString(ShellEnvironmentStoreProvider.java:155)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreEntryWrapper.update(StoreEntryWrapper.java:228)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreViewState.lambda$updateContent$1(StoreViewState.java:147)<br>
> at
java.lang.Iterable.forEach(Iterable.java:75)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreViewState.updateContent(StoreViewState.java:147)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreViewState.init(StoreViewState.java:93)<br>
> at <br>
>
io.xpipe.app.core.mode.BaseMode.lambda$onSwitchTo$1(BaseMode.java:109)<br>
> at
io.xpipe.app.util.ThreadHelper.lambda$load$0(ThreadHelper.java:78)<br>
> at java.lang.Thread.run(Thread.java:1447)<br>
><br>
> 21:25:38:847 - error: Index 3 out of bounds for
length 2<br>
> java.lang.ArrayIndexOutOfBoundsException: Index
3 out of bounds for <br>
> length 2<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:248)<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper$Generic.addListener(ExpressionHelper.java:200)<br>
> at <br>
>
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:65)<br>
> at <br>
>
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:86)<br>
> at
javafx.beans.binding.StringBinding.bind(StringBinding.java:114)<br>
> at
javafx.beans.binding.Bindings$7.<init>(Bindings.java:428)<br>
> at <br>
>
javafx.beans.binding.Bindings.createStringBinding(Bindings.java:426)<br>
> at <br>
>
io.xpipe.app.util.StoreStateFormat.shellEnvironment(StoreStateFormat.java:24)<br>
> at <br>
>
io.xpipe.ext.proc.env.ShellEnvironmentStoreProvider.informationString(ShellEnvironmentStoreProvider.java:155)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreEntryWrapper.update(StoreEntryWrapper.java:228)<br>
> at <br>
>
io.xpipe.app.comp.store.StoreEntryWrapper.lambda$setupListeners$3(StoreEntryWrapper.java:143)<br>
> at <br>
>
io.xpipe.app.util.PlatformThread.lambda$runLaterIfNeeded$0(PlatformThread.java:318)<br>
> at <br>
>
com.sun.javafx.application.PlatformImpl.lambda$runLater$4(PlatformImpl.java:424)<br>
> at <br>
>
com.sun.glass.ui.InvokeLaterDispatcher$Future.run$$$capture(InvokeLaterDispatcher.java:95)<br>
> at <br>
>
com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java)<br>
><br>
> This full log goes up to index 50 out of bounds
due to the recurring <br>
> nature of this exception.<br>
><br>
> Looking at the implementation of
ExpressionHelper, I don't see any <br>
> harm in just synchronizing the methods, at
least from my perspective. <br>
> But I guess that is up to the developers to
decide. The only real <br>
> solution I have as an application developer is
to perform all <br>
> initialization on one thread or just hope that
this error is rare <br>
> enough, both of which aren't great options. So
I hope that a potential <br>
> synchronization of the ExpressionHelper methods
can be considered.<br>
><br>
> Best<br>
> Christopher Schnick<br>
><o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</body>
</html>