RFR: 8377850: AbstractPrimaryTimer: concurrent addAnimationTimer() causes lost timers and hangs

[Andy Goryachev]

On Thu, 12 Feb 2026 23:03:31 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

> Add `synchronized(lock)` around array mutations in `addPulseReceiver`/`removePulseReceiver`/`addAnimationTimer`/`removeAnimationTimer` and around snapshot-taking in `timePulseImpl`. Iteration remains outside the lock. `updateAnimationRunnable()` is also called outside the lock to avoid nested locking.
> 
> This preserves the existing copy-on-write design - the lock just ensures it works correctly across threads. Performance impact is minimal: the lock only covers field reads/writes, not the per-frame iteration.
> 
> **Testing:**
> New `AbstractPrimaryTimerThreadSafetyTest` with `testConcurrentAddAnimationTimer` - 8 threads add timers simultaneously, repeated 100 times. Fails 100% without fix. Reuses `AbstractPrimaryTimerStub` from existing tests.
> 
> All existing animation tests pass.
> 
> In JPro, this often caused a Deadlock during startup.
> This might have caused many bugs, which are very hard to reproduce.

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/AbstractPrimaryTimer.java line 82:

> 80:             case ANIMATION_MBEAN_ENABLED:
> 81:                 AnimationPulse.getDefaultBean()
> 82:                         .setEnabled(Settings.getBoolean(ANIMATION_MBEAN_ENABLED));

unnecessary change

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/AbstractPrimaryTimer.java line 101:

> 99:     @SuppressWarnings("unchecked")
> 100:     private ReceiverRecord<TimerReceiver>[] animationTimers = new ReceiverRecord[2]; // frameJobList
> 101:     // snapshot

unnecessary change

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/AbstractPrimaryTimer.java line 426:

> 424:                 if (Logging.getJavaFXLogger().isLoggable(System.Logger.Level.WARNING)) {
> 425:                     Logging.getJavaFXLogger().warning(
> 426:                             "Too many exceptions thrown by " + type().getSimpleName() + ", ignoring further exceptions.");

unnecessary change

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2074#discussion_r2806483946
PR Review Comment: https://git.openjdk.org/jfx/pull/2074#discussion_r2806484225
PR Review Comment: https://git.openjdk.org/jfx/pull/2074#discussion_r2806483678