On Tue, 4 Mar 2025 19:34:11 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> @kevinrushforth I don't know, I would think it should be possible to create
>> a property or property wrapper to make it thread-safe. Just having a
>> version with all its methods synchronized is probably sufficient. If that
>> saves a lot of complex logic in, what looks like, dozens of controls, then
>> it seems like a far better solution to me. Of course, that should only
>> apply to special properties; I'm not suggesting making all properties
>> synchronized -- although I think we could as, without contention (since
>> they're supposed to be used from a single thread), it would be nearly free,
>> and it would make all kinds of undefined behavior scenario's when used from
>> multiple threads (by accident or otherwise) suddenly deterministic and
>> debuggable.
>
> I think the problem is in the callbacks themselves, as they'd be on the wrong
> thread. I vaguely remember experimenting with a system where you can provide
> an Executor for callbacks (like `Platform::runLater`) to mitigate this.
The listener callbacks will always be on the thread that mutates the property
being listened to, so that isn't the problem. The problem in this case is that
adding or removing a listener while another thread is modifying the property,
which will iterate the list of listeners, is not thread-safe. Consider the
following:
final var prop = new SimpleBooleanProperty(false);
// add listeners in a background thread
var thr = new Thread(() -> {
for (int i = 0; i < 10000; i++) {
ChangeListener<Boolean> listener = (obs, o, n) -> {
if (!mainThread.equals(Thread.currentThread())) {
System.err.println("***** ERROR: listener called on
wrong thread: " + Thread.currentThread());
}
};
prop.addListener(listener);
}
});
thr.start();
// Fire events in main thread
for (int jj = 0; jj < 10000; jj++) {
prop.set(!prop.get());
}
The listeners all get called on the (correct) main thread, but continually
adding listeners while the property is modified will lead to AIOOBE or NPE (the
above example gets random NPEs when I run it).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1980254928
