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

[Kevin Rushforth]

On Tue, 4 Mar 2025 19:34:11 GMT, John Hendrikx <jhendrikx at 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