On Thu, 30 Jan 2025 23:35:05 GMT, Andy Goryachev <ango...@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
