On Mon, 14 Apr 2025 12:44:42 GMT, Michael Strauß <[email protected]> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PreferenceProperties.java
>> line 246:
>>
>>> 244: }
>>> 245:
>>> 246: public synchronized void fireValueChangedIfNecessary() {
>>
>> I noticed that you made many methods `synchronized` in this class, but I
>> have trouble seeing why this is. A short explanation may be in order.
>>
>> So, if I had to guess: it's possible to change the value override and/or the
>> updating of all properties from a different thread, which is not the FX
>> thread. However, what guarantees are now given to listeners (if any)? The
>> value changed events can now also be triggered from any thread, unless
>> wrapped in a `Platform.runLater`.
>>
>> What I mean here is, let's say I listen on `accentColorProperty` of
>> `Platform.Preferences`, on what thread can the change event happen? The
>> `Platform.Preferences` class does not mention anything special, so I think
>> it is fair to assume it should be on the FX thread, allowing me to make
>> modifications to an active scene graph in my change handler; however that
>> will fail if the change notification was not triggered from the FX thread.
>
> Platform preference change events always happen on the FX thread. The reason
> for `synchronized` is as follows:
>
> A `Scene` can be created on any thread as per spec. When the scene is
> created, the `ScenePreferences` class and its associated properties are
> instantiated, which in turn subscribe internally to the
> `Platform.Preferences` properties. The synchronization is to protect the
> addition and removal of listeners.
>
> This is not ideal, though. As soon as we're subscribed to
> `Platform.Preferences`, the `Scene.Preferences` can receive change
> notifications on the FX thread, which interferes with the mandated capability
> to be created on any thread.
>
> I think we have a few options here:
> 1. Don't subscribe scene preferences to platform preferences until the scene
> is shown. The downside of this is that we don't know the actual values of the
> preferences up until that point.
> 2. With limited synchronization, fetch the current preference values from the
> platform when the scene is created, but don't subscribe to change
> notifications until the scene is shown.
> 3. Specify that scene preference changes can happen on the FX thread, even
> when the scene is created in a background thread.
I've decided to go with option 2.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1655#discussion_r2047234821
