RFR: 8351067: Enforce Platform threading use [v4]
Kevin Rushforth
kcr at openjdk.org
Thu Mar 13 15:26:09 UTC 2025
On Mon, 10 Mar 2025 20:30:43 GMT, Andy Goryachev <angorya at 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
More information about the openjfx-dev
mailing list