Hi,
Interesting idea. We have this problem specially when Junior
developers touch the code.
The other way around would be nice too - if some I/O task
executes on the FX thread.
This can make the OS think the application hanged and offer to
kill it, since it won't respond to "pings". And I/O tasks
processing time may vary between installations. Also causes
"white screens" since it blocks painting.
-- Thiago.
Em seg., 5 de ago. de 2024 às 11:59, Kevin Rushforth
<kevin.rushfo...@oracle.com> escreveu:
Wouldn't it be better to implement this check in assert to
avoid any impact in production?
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.
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.
The things to consider would be:
1. What is the performance hit of doing this check on the
setting of every property?
2. What is the effect on bound properties?
3. How intrusive is it in the code?
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?
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.
-- Kevin
On 8/4/2024 8:40 PM, quizynox wrote:
Hello,
Wouldn't it be better to implement this check in assert to
avoid any impact in production?
пн, 5 авг. 2024 г. в 03:30, John Hendrikx
<john.hendr...@gmail.com>:
Hi list,
I know of quite some bugs and users that have been
bitten by the
threading model used by JavaFX. Basically, anything
directly or
indirectly linked to an active Scene must be accessed on
the FX thread.
However, as FX also allows manipulating nodes and
properties before
they're displayed, there can be no "hard" check
everywhere to ensure we
are on the FX thread (specifically, in properties).
Now, I think this situation is annoying, as a simple
mistake where a
Platform.runLater wrapper was forgotten usually results
in programs
operating mostly flawlessly, but then fail in mysterious
and random and
hard to reproduce ways. The blame is often put on FX as
the resulting
exceptions will almost never show the user code which
was the actual
culprit. It can result in FX being perceived as
unstable or buggy.
So I've been thinking if there isn't something we can do
to detect these
bugs originating from user code much earlier, similar to
the
`ConcurrentModificationException` the collection classes
do when
accessed in nested or concurrent contexts.
I think it may be possible to have properties check
whether they're part
of an active scene without too much of an performance
impact, possibly
even behind a switch. It would work like this:
Properties involved with Nodes will have an associated
bean instance
(`getBean`). This is an object, but we could check here
if this
instance implements an interface:
if (getBean() instanceof MayBePartOfSceneGraph x) {
if (x.isPartOfActiveScene() &&
!isOnFxThread()) {
throw new
IllegalStateException("Property must only be
used from the FX Application Thread");
}
}
This check could be done on every set of the property,
and potentially
on every get as well. It should be relatively cheap,
but will expose
problematic code patterns at a much earlier stage.
There's a chance
that this will "break" some programs that seemed to be
behaving
correctly as well, so we may want to put it behind a
switch until such
programs (or libraries) can be fixed.
What do you all think?
--John
(*) Names of methods/interfaces are only used for
illustration purposes,
we can think of good names if this moves forward.
