On Fri, 16 May 2025 18:12:35 GMT, Michael Strauß <[email protected]> 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 380:
> 378: Thread thread = Thread.currentThread();
> 379:
> thread.getUncaughtExceptionHandler().uncaughtException(thread, e);
> 380: } catch (Throwable ignored) {
any reason not to wrap the whole method in `try`/`catch`?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1811#discussion_r2093508251
