RFR: 8347392: Thread-unsafe implementation of c.s.j.scene.control.skin.Utils

[Kevin Rushforth]

On Thu, 30 Jan 2025 23:35:05 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> Thread-safe and re-entrant implementation of Utils.
> 
> The new code still uses the static instances of Text and TextLayout for performance reasons, but adds a thread-safe mechanism to keep track of whether any of the instances is in use and when that happens, supplies a new instance instead.  This solves both thread safety and re-entrancy.

The fix looks good. My testing is good as well. The now-enabled tests fail pretty reliably for me on Windows without your fix and all pass with your fix.

I left a few minor questions / comments, but will approve as is.

tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 249:

> 247: 
> 248:     @Test
> 249:     public void checkBox() {

Minor: this test calls `setSelected(true)`. Would introducing randomness via `setSelected(nextBoolean())` be useful here?

tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 263:

> 261: 
> 262:     @Test
> 263:     public void choiceBox() {

Minor: select random item?

tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 330:

> 328: 
> 329:     @Test
> 330:     public void hyperlink() {

Minor: `setVisited(nextBoolean())`?

-------------

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1691#pullrequestreview-2600260514
PR Review Comment: https://git.openjdk.org/jfx/pull/1691#discussion_r1945619434
PR Review Comment: https://git.openjdk.org/jfx/pull/1691#discussion_r1945620100
PR Review Comment: https://git.openjdk.org/jfx/pull/1691#discussion_r1945618566