On Fri, 16 May 2025 18:12:35 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> When an exception is thrown from `AnimationTimer::handle`, the JavaFX
>> application freezes. The reason is that the user exception will bubble up
>> into framework code, preventing the normal operation of JavaFX.
>>
>> The following program demonstrates the defect:
>>
>>
>> public class FailingAnimationTimer extends Application {
>> @Override
>> public void start(Stage stage) throws Exception {
>> var button = new Button("start timer");
>> button.setOnAction(_ -> {
>> var timer = new AnimationTimer() {
>> @Override
>> public void handle(long l) {
>> throw new RuntimeException("foo");
>> }
>> };
>>
>> timer.start();
>> });
>>
>> var root = new HBox();
>> root.getChildren().add(new TextField("test"));
>> root.getChildren().add(button);
>> stage.setScene(new Scene(root));
>> stage.show();
>> }
>> }
>>
>>
>> The solution is to not allow user exceptions to bubble up into animation
>> framework code. If an exception occurs, it is instead sent to the current
>> thread's uncaught exception handler. This is the same thing that we already
>> do for exceptions thrown by invalidation listeners and change listeners.
>>
>> In addition to that, a failing animation timer has the potential to spam
>> logs, which is why I introduced a cut-off value at 100 exceptions for each
>> individual timer, after which no further exceptions from this particular
>> timer are sent to the uncaught exception handler. After reaching the cut-off
>> value, the following warning is logged:
>>
>> `WARNING: Too many exceptions thrown by AnimationTimer, ignoring further
>> exceptions. The cut-off number can be set with the system property
>> com.sun.scenario.animation.failingTimerThreshold (current = 100).`
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
modules/javafx.graphics/src/main/java/com/sun/scenario/animation/AbstractPrimaryTimer.java
line 352:
> 350: }
> 351:
> 352: private static abstract class ReceiverRecord<T> {
All this does look complicated, just to limit the number of messages in the log
(a noble cause).
What if we just print the first message and ignore the rest? It might be
relatively unlikely that two different failures appeared at the same time, each
with its own root cause?
If we just show the first one, we'll remove all this complexity and the system
property, and possibly get back to the simpler original code?
What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1811#discussion_r2093530134
