On Mon, 31 Jul 2023 17:39:29 GMT, Jose Pereda <jper...@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). > > I've edited the title of the original JBS issue > https://bugs.openjdk.org/browse/JDK-8159048 to fix the typo. @jperedadnr One more thing I noticed: the titles of the bug and the CSR should match (it's not enforced, but it a best practice). So you should either change the title of the CSR back to match the title of the bug -- "Null PulseReceiver in AbstractMasterTimer" -- or change the title of the bug, along with this PR title, to match the CSR. In looking at it, I think the CSR title is more descriptive, so I might suggest some variant of that. Here a slightly shortened suggestion: Animation and AnimationTimer methods must be called on JavaFX Application thread but what you have is fine as well. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1167#issuecomment-1677358792
