<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<blockquote type="cite">Wouldn't it be better to implement this
check in assert to avoid any impact in production?</blockquote>
<br>
No. Using an assert in a case like this is an anti-pattern. A call
to assert in a library such as JavaFX is only appropriate for
checking an invariant in internal logic. If we are going to go down
this route of doing a thread check when mutating properties of
"live" nodes, we will throw the same IllegalStateException that is
currently thrown by some methods on Stage and Scene.<br>
<br>
As for the proposal itself, adding this check is an interesting
idea. We considered doing this back in the JDK 7 (JavaFX 2) time
frame, but decided not to pursue it then. I think the idea is worth
further discussion. I would limit any thread checking to setting the
property. It would be too restrictive (and largely unnecessary) to
prevent reading a property from the application thread.<br>
<br>
The things to consider would be:<br>
<br>
1. What is the performance hit of doing this check on the setting of
every property?<br>
2. What is the effect on bound properties?<br>
3. How intrusive is it in the code?<br>
4. Should we add a property to enable / disable the thread check,
possibly a three- or four-valued property (allow|warn|debug?|deny),
as was recently done in JEP 471 for sun.misc.Unsafe memory access
methods. If so, what should the default be?<br>
<br>
My quick take is that if this can be done in a minimally intrusive
manner with low overhead, we should consider pursing this. As for 4,
my preference would be to add a three- or four-valued system
property to control the check, with "warn" as the default initially,
changing the default to "disallow" in a subsequent version. This
would, of course, require a lot of testing.<br>
<br>
-- Kevin<br>
<br>
<br>
<div class="moz-cite-prefix">On 8/4/2024 8:40 PM, quizynox wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAASO9vSabGPOVjh7dpGm=Z+SiWshWZS7fb2kb7afrsrfLAfOyA@mail.gmail.com">
<div dir="ltr">Hello,<br>
<br>
Wouldn't it be better to implement this check in assert to avoid
any impact in production?</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">пн, 5 авг. 2024 г. в 03:30,
John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" moz-do-not-send="true" class="moz-txt-link-freetext">john.hendrikx@gmail.com</a>>:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi
list,<br>
<br>
I know of quite some bugs and users that have been bitten by
the <br>
threading model used by JavaFX. Basically, anything directly
or <br>
indirectly linked to an active Scene must be accessed on the
FX thread. <br>
However, as FX also allows manipulating nodes and properties
before <br>
they're displayed, there can be no "hard" check everywhere to
ensure we <br>
are on the FX thread (specifically, in properties).<br>
<br>
Now, I think this situation is annoying, as a simple mistake
where a <br>
Platform.runLater wrapper was forgotten usually results in
programs <br>
operating mostly flawlessly, but then fail in mysterious and
random and <br>
hard to reproduce ways. The blame is often put on FX as the
resulting <br>
exceptions will almost never show the user code which was the
actual <br>
culprit. It can result in FX being perceived as unstable or
buggy.<br>
<br>
So I've been thinking if there isn't something we can do to
detect these <br>
bugs originating from user code much earlier, similar to the <br>
`ConcurrentModificationException` the collection classes do
when <br>
accessed in nested or concurrent contexts.<br>
<br>
I think it may be possible to have properties check whether
they're part <br>
of an active scene without too much of an performance impact,
possibly <br>
even behind a switch. It would work like this:<br>
<br>
Properties involved with Nodes will have an associated bean
instance <br>
(`getBean`). This is an object, but we could check here if
this <br>
instance implements an interface:<br>
<br>
if (getBean() instanceof MayBePartOfSceneGraph x) {<br>
if (x.isPartOfActiveScene() &&
!isOnFxThread()) {<br>
throw new IllegalStateException("Property
must only be <br>
used from the FX Application Thread");<br>
}<br>
}<br>
<br>
This check could be done on every set of the property, and
potentially <br>
on every get as well. It should be relatively cheap, but will
expose <br>
problematic code patterns at a much earlier stage. There's a
chance <br>
that this will "break" some programs that seemed to be
behaving <br>
correctly as well, so we may want to put it behind a switch
until such <br>
programs (or libraries) can be fixed.<br>
<br>
What do you all think?<br>
<br>
--John<br>
<br>
(*) Names of methods/interfaces are only used for illustration
purposes, <br>
we can think of good names if this moves forward.<br>
<br>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>