On Tue, 4 Mar 2025 23:00:01 GMT, John Hendrikx <[email protected]> wrote:
>> 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());
>> }
>> }
>> }
>
> Of course for a generic solution, we could provide a wrapper for properties,
> or custom synchronized properties.
Yes, using a custom property, or a wrapper, would solve this particular
synchronization problem, although that wouldn't solve all of the problems.
We would then be left with the problem that if the accessibility change
listener did fire -- which necessarily happens on the FX app thread -- while
the object was being set up on a background thread, the listener might try to
modify properties that are being touched on the background thread. This is just
another case of a more general problem we already have with animation, and
which Andy fixed by not starting animation in a few places unless and until we
are on the FX app thread.
So I think the solution Andy has chosen of deferring adding the listener is
better than trying to fix the accessibility property, followed by fixing the
listeners that use it, to be thread-safe.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1980469351
