RFR: 8159048: Null PulseReciever in AbstractMasterTimer

Andy Goryachev angorya at openjdk.org
Mon Jul 31 17:15:50 UTC 2023


On Mon, 3 Jul 2023 15:49:31 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

> This PR adds a check to the Animation and AnimationTimer public methods to verify that these are called from the JavaFX Application thread. If the call is done from any other thread, an IllegalStateException will be thrown.
> 
> This will prevent users from getting unexpected errors (typically NPE, like the one posted in the JBS issue), and will fail fast with a clear exception and reason for it.
> 
> The javadoc of the Animation and AnimationTimer classes and public methods has been updated accordingly.
> 
> Tests for both classes have been included, failing (as in no exceptions were thrown when calling from a background thread) before this patch, and passing (as in ISE was thrown).

modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line 990:

> 988:      */
> 989:     public void play() {
> 990:         Toolkit.getToolkit().checkFxUserThread();

minor: perhaps this should first check for non-null parent, then for fx thread (here and below)

tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java line 62:

> 60:             startupLatch.countDown();
> 61:         }
> 62: 

minor: extra newline?

tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java line 82:

> 80:                 @Override public void handle(long l) {
> 81:                     frameCounter.countDown();
> 82:                     if (frameCounter.getCount() == 0L) {

thank you for 0L!

tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java line 112:

> 110:         Platform.runLater(timer::start);
> 111:     }
> 112: 

minor: extra newline

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279641091
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642701
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642982
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642236


More information about the openjfx-dev mailing list