Fix animation callback crashes from iterator invalidation (#119)

Resolved segfaults caused by creating new animations from within
animation callbacks. The issue was iterator invalidation in
AnimationManager::update() when callbacks modified the active
animations vector.

Changes:
- Add deferred animation queue to AnimationManager
- New animations created during update are queued and added after
- Set isUpdating flag to track when in update loop
- Properly handle Animation destructor during callback execution
- Add clearCallback() method for safe cleanup scenarios

This fixes the "free(): invalid pointer" and "malloc(): unaligned
fastbin chunk detected" errors that occurred with rapid animation
creation in callbacks.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/Animation.cpp

@@ -1,6 +1,8 @@
 #include "Animation.h"
 #include "UIDrawable.h"
 #include "UIEntity.h"
+#include "PyAnimation.h"
+#include "McRFPy_API.h"
 #include <cmath>
 #include <algorithm>
 #include <unordered_map>
@@ -9,6 +11,11 @@
 #define M_PI 3.14159265358979323846
 #endif
 
+// Forward declaration of PyAnimation type
+namespace mcrfpydef {
+    extern PyTypeObject PyAnimationType;
+}
+
 // Animation implementation
 Animation::Animation(const std::string& targetProperty, 
                      const AnimationValue& targetValue,
@@ -30,10 +37,13 @@ Animation::Animation(const std::string& targetProperty,
 }
 
 Animation::~Animation() {
-    // Decrease reference count for Python callback
+    // Decrease reference count for Python callback if we still own it
-    if (pythonCallback) {
+    PyObject* callback = pythonCallback;
+    if (callback) {
+        pythonCallback = nullptr;
+        
         PyGILState_STATE gstate = PyGILState_Ensure();
-        Py_DECREF(pythonCallback);
+        Py_DECREF(callback);
         PyGILState_Release(gstate);
     }
 }
@@ -43,6 +53,7 @@ void Animation::start(std::shared_ptr<UIDrawable> target) {
     
     targetWeak = target;
     elapsed = 0.0f;
+    callbackTriggered = false; // Reset callback state
     
     // Capture start value from target
     std::visit([this, &target](const auto& targetVal) {
@@ -93,6 +104,7 @@ void Animation::startEntity(std::shared_ptr<UIEntity> target) {
     
     entityTargetWeak = target;
     elapsed = 0.0f;
+    callbackTriggered = false; // Reset callback state
     
     // Capture the starting value from the entity
     std::visit([this, target](const auto& val) {
@@ -118,6 +130,19 @@ bool Animation::hasValidTarget() const {
     return !targetWeak.expired() || !entityTargetWeak.expired();
 }
 
+void Animation::clearCallback() {
+    // Safely clear the callback when PyAnimation is being destroyed
+    PyObject* callback = pythonCallback;
+    if (callback) {
+        pythonCallback = nullptr;
+        callbackTriggered = true; // Prevent future triggering
+        
+        PyGILState_STATE gstate = PyGILState_Ensure();
+        Py_DECREF(callback);
+        PyGILState_Release(gstate);
+    }
+}
+
 void Animation::complete() {
     // Jump to end of animation
     elapsed = duration;
@@ -165,9 +190,9 @@ bool Animation::update(float deltaTime) {
     }
     
     // Trigger callback when animation completes
+    // Check pythonCallback again in case it was cleared during update
     if (isComplete() && !callbackTriggered && pythonCallback) {
         triggerCallback();
-        callbackTriggered = true;
     }
     
     return !isComplete();
@@ -319,13 +344,14 @@ void Animation::applyValue(UIEntity* entity, const AnimationValue& value) {
 void Animation::triggerCallback() {
     if (!pythonCallback) return;
     
+    // Ensure we only trigger once
+    if (callbackTriggered) return;
+    callbackTriggered = true;
+    
     PyGILState_STATE gstate = PyGILState_Ensure();
     
-    // We need to create PyAnimation wrapper for this animation
+    // TODO: In future, create PyAnimation wrapper for this animation
-    // and PyObject wrapper for the target
+    // For now, pass None for both parameters
-    // For now, we'll pass None for both as a simple implementation
-    // TODO: In future, wrap the animation and target objects properly
-    
     PyObject* args = PyTuple_New(2);
     Py_INCREF(Py_None);
     Py_INCREF(Py_None);
@@ -338,6 +364,7 @@ void Animation::triggerCallback() {
     if (!result) {
         // Print error but don't crash
         PyErr_Print();
+        PyErr_Clear(); // Clear the error state
     } else {
         Py_DECREF(result);
     }
@@ -594,11 +621,19 @@ AnimationManager& AnimationManager::getInstance() {
 
 void AnimationManager::addAnimation(std::shared_ptr<Animation> animation) {
     if (animation && animation->hasValidTarget()) {
-        activeAnimations.push_back(animation);
+        if (isUpdating) {
+            // Defer adding during update to avoid iterator invalidation
+            pendingAnimations.push_back(animation);
+        } else {
+            activeAnimations.push_back(animation);
+        }
     }
 }
 
 void AnimationManager::update(float deltaTime) {
+    // Set flag to defer new animations
+    isUpdating = true;
+    
     // Remove completed or invalid animations
     activeAnimations.erase(
         std::remove_if(activeAnimations.begin(), activeAnimations.end(),
@@ -607,6 +642,17 @@ void AnimationManager::update(float deltaTime) {
             }),
         activeAnimations.end()
     );
+    
+    // Clear update flag
+    isUpdating = false;
+    
+    // Add any animations that were created during update
+    if (!pendingAnimations.empty()) {
+        activeAnimations.insert(activeAnimations.end(), 
+                              pendingAnimations.begin(), 
+                              pendingAnimations.end());
+        pendingAnimations.clear();
+    }
 }
 
 

src/Animation.h

@@ -62,6 +62,9 @@ public:
     // Check if animation has valid target
     bool hasValidTarget() const;
     
+    // Clear the callback (called when PyAnimation is deallocated)
+    void clearCallback();
+    
     // Animation properties
     std::string getTargetProperty() const { return targetProperty; }
     float getDuration() const { return duration; }
@@ -83,8 +86,9 @@ private:
     std::weak_ptr<UIEntity> entityTargetWeak;
     
     // Callback support
-    PyObject* pythonCallback = nullptr;  // Python callback function
+    PyObject* pythonCallback = nullptr;  // Python callback function (we own a reference)
     bool callbackTriggered = false;      // Ensure callback only fires once
+    PyObject* pyAnimationWrapper = nullptr; // Weak reference to PyAnimation if created from Python
     
     // Helper to interpolate between values
     AnimationValue interpolate(float t) const;
@@ -163,4 +167,6 @@ public:
 private:
     AnimationManager() = default;
     std::vector<std::shared_ptr<Animation>> activeAnimations;
+    std::vector<std::shared_ptr<Animation>> pendingAnimations; // Animations to add after update
+    bool isUpdating = false; // Flag to track if we're in update loop
 };