On Mon, 10 Mar 2025 20:30:43 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Summary >> ------- >> >> Adds a thread check to a number of `Platform` methods: >> >> accessibilityActiveProperty() >> getPreferences() >> isAccessibilityActive() >> >> These methods will throw an `IllegalStateException` if called on a thread >> other than the JavaFX Application Thread. >> >> Problem >> ------- >> >> JavaFX allows the Nodes and Scenes to be created and modified on any thread >> as long as they are not yet attached to a `Window` that is showing. >> >> This is allowed in an implicit assumption that the construction code only >> modifies the properties of the said Nodes and Scenes, but not other static >> or global entities. Concurrent multi-threaded access of such entities not >> only breaks the initialization of the properties, but also causes the >> failures down the road, if the change to the global properties happens while >> the Nodes and Scenes are still being constructed. >> >> Even JavaFX platform developers did not avoid tripping over this issue, as >> can be illustrated by https://bugs.openjdk.org/browse/JDK-8348987 . >> >> Solution >> -------- >> >> Fail each method fast with an `IllegalStateException` if called from a >> background thread. >> >> While this solution won't prevent other possible abuse, such as getting a >> reference to the property in the JavaFX Application Thread and later >> accessing it in a background thread, adding a check and allowing these >> methods to fail fast should prevent most likely scenarios. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > get preferences Looks good. I left one minor comment on the test. tests/system/src/test/java/test/javafx/application/PlatformTest.java line 129: > 127: > 128: @Test > 129: public void accessibilityActiveNotOnFxThread() { Minor: Since you now also test `Platform::getPreferences` you might consider renaming this test method. ------------- Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1728#pullrequestreview-2682396958 PR Review Comment: https://git.openjdk.org/jfx/pull/1728#discussion_r1993769589
