On Tue, 4 Mar 2025 21:24:46 GMT, Kevin Rushforth <[email protected]> wrote:
>> 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).
Yes, but that's because there is no synchronization. Here is a version that
does do synchronization; I see no more exceptions (and it runs just as fast
really):
public class PropTest {
public static void main(String[] args) {
Thread mainThread = Thread.currentThread();
final var prop = new SimpleBooleanProperty(false) {
@Override
public synchronized void addListener(ChangeListener<? super Boolean>
listener) {
super.addListener(listener);
}
@Override
public synchronized void set(boolean newValue) {
super.set(newValue);
}
};
// add listeners in a background thread
var thr = new Thread(() -> {
for (int i = 0; i < 1000000; 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 < 1000000; jj++) {
prop.set(!prop.get());
}
}
}
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1980380695
