RFR: 8349091: Charts: exception initializing in a background thread [v6]

[Kevin Rushforth]

On Tue, 4 Mar 2025 23:00:01 GMT, John Hendrikx <jhendrikx at openjdk.org> 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