On Sat, 17 May 2025 12:24:46 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:
> 
>   remove system property

Looks reasonable although not having a way to override a threshold is strange. 
OTOH 100 is a generous enough for the application.

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

Marked as reviewed by kizune (Author).

PR Review: https://git.openjdk.org/jfx/pull/1811#pullrequestreview-2859849186

Reply via email to