john/McRogueFace

Fork 0

[Bugfix] Fix segfault when animation callbacks start new animations #241

New issue

Closed

opened 2026-02-04 12:53:06 +00:00 by john · 2 comments

john commented

2026-02-04 12:53:06 +00:00

Owner

Problem

Animation callbacks that started new animations on the same property caused a segfault in AnimationManager::update(). This was a classic iterator invalidation bug.

Root Cause

When an animation completes and its callback fires, if the callback starts a new animation with REPLACE conflict mode (the default), the code would:

Call existingAnim->complete() on the old animation
Call activeAnimations.erase() to remove the old animation

But this erase happened while remove_if was still iterating over activeAnimations, invalidating the iterator and causing a crash.

Reproduction

def recursive_callback(target, prop, value):
    target.animate("x", 200, 0.1, callback=recursive_callback)

frame.animate("x", 100, 0.1, callback=recursive_callback)

Solution

Modified AnimationManager::addAnimation() to handle the isUpdating flag properly:

During update loop (isUpdating=true): Call stop() instead of complete() on the existing animation, and let the update loop clean it up naturally. This avoids triggering the old animation's callback (which could cause infinite recursion) and avoids iterator invalidation.
Outside update loop (isUpdating=false): Preserve original behavior - call complete() and erase directly.

Also fixed the pending animations processing to recognize when an animation is already the property lock holder (which happens when addAnimation is called during a callback).

Testing

Added regression test: tests/regression/recursive_animation_callback_segfault.py
Verified test_animation_property_locking.py still passes (8/8 tests)
Verified animate_method_test.py still passes
Verified test_animation_callback_simple.py still passes

Files Changed

src/Animation.cpp - Modified addAnimation() and pending animations processing

## Problem Animation callbacks that started new animations on the same property caused a segfault in `AnimationManager::update()`. This was a classic iterator invalidation bug. ### Root Cause When an animation completes and its callback fires, if the callback starts a new animation with `REPLACE` conflict mode (the default), the code would: 1. Call `existingAnim->complete()` on the old animation 2. Call `activeAnimations.erase()` to remove the old animation But this erase happened **while `remove_if` was still iterating** over `activeAnimations`, invalidating the iterator and causing a crash. ### Reproduction ```python def recursive_callback(target, prop, value): target.animate("x", 200, 0.1, callback=recursive_callback) frame.animate("x", 100, 0.1, callback=recursive_callback) ``` ## Solution Modified `AnimationManager::addAnimation()` to handle the `isUpdating` flag properly: 1. **During update loop (`isUpdating=true`)**: Call `stop()` instead of `complete()` on the existing animation, and let the update loop clean it up naturally. This avoids triggering the old animation's callback (which could cause infinite recursion) and avoids iterator invalidation. 2. **Outside update loop (`isUpdating=false`)**: Preserve original behavior - call `complete()` and erase directly. Also fixed the pending animations processing to recognize when an animation is already the property lock holder (which happens when `addAnimation` is called during a callback). ## Testing - Added regression test: `tests/regression/recursive_animation_callback_segfault.py` - Verified `test_animation_property_locking.py` still passes (8/8 tests) - Verified `animate_method_test.py` still passes - Verified `test_animation_callback_simple.py` still passes ## Files Changed - `src/Animation.cpp` - Modified `addAnimation()` and pending animations processing

john added the

Tiny Feature

system:ui-hierarchy

labels

2026-02-04 12:53:16 +00:00

john removed the

system:ui-hierarchy

Tiny Feature

labels

2026-02-07 18:41:31 +00:00

john commented

2026-02-07 19:16:20 +00:00

Author

Owner

486087b9cb

486087b9cb

john closed this issue

2026-02-07 19:16:20 +00:00

john commented

2026-02-07 19:24:56 +00:00

Author

Owner

Fixed by commit d2ea64b ("fix: animations modifying animations during callback is now safe"). Regression test added at tests/regression/recursive_animation_callback_segfault.py.

Fixed by commit d2ea64b ("fix: animations modifying animations during callback is now safe"). Regression test added at `tests/regression/recursive_animation_callback_segfault.py`.

john added the

Bugfix

system:animation

priority:tier1-active

labels

2026-02-07 19:25:50 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

john/McRogueFace#241

No description provided.

Rows
Columns