Modifying activeAnimations while enumerating through it

[Naxum]

Howdy! Not sure if I'm going about this in a weird way, but I must be somehow calling the addAnimation/removeAnimationWithID while tickActiveAnimations is being called, making my app crash.

My basic solution is to make two more collections that modify activeAnimations after it's done looping.

@property (nonatomic, strong) NSMutableArray *animationsToAdd;
@property (nonatomic, strong) NSMutableDictionary *animationsToRemove;

Insert/remove the animations immediately after the for loop in tickActiveAnimations

for (JotINTUAnimation *anim in self.animationsToAdd) {
    anim.startTime = CACurrentMediaTime();
    [self.activeAnimations setObject:anim forKey:@(anim.animationID)];
}

for (NSNumber *animationID in [self.animationsToRemove keyEnumerator]) {
    JotINTUAnimation *animation = [self.activeAnimations objectForKey:animationID];
    [animation complete:[[self.animationsToRemove objectForKey:animationID] boolValue]];
    [self.activeAnimations removeObjectForKey:animationID];
    if ([self.activeAnimations count] == 0) {
        self.displayLink.paused = YES;
    }
}

[self.animationsToRemove removeAllObjects];
[self.animationsToAdd removeAllObjects];

Rewrite the basic addAnimation and removeAnimationWithID methods

- (void)addAnimation:(JotINTUAnimation *)animation
{
    if ([self.activeAnimations count] == 0) {
        self.displayLink.paused = NO;
    }

    [self.animationsToAdd addObject:animation];
}

- (void)removeAnimationWithID:(JotINTUAnimationID)animationID didFinish:(BOOL)finished
{
    [self.animationsToRemove setObject:@(finished) forKey:@(animationID)];
}

This is my first issue I've ever created, if you want me to submit a PR let me know. Also, if I am totally overengineering this problem, let me know that as well :D

[smileyborg]

Hi Jake!

Thanks for reporting this. You might be on to something here. I'm not completely sure what is causing the crash you're seeing just based on your description though. Do you have any code that reproduces the issue? Or can you modify the example project in this repo to reproduce the crash?

I want to make sure I understand what's happening before we get into how to fix it. (If I had to guess right now, it seems as though this may be caused by accessing INTUAnimationEngine from multiple threads. Because in theory, the CADisplayLink is calling back on the main thread, and if INTUAnimationEngine is only used from the main thread, then it should not be possible to add/remove an animation while inside tickActiveAnimations.)

[Naxum]

I made a quick theory project to highlight the problem.

The error happens when you start a new animation from within an animation closure.

Basically, I have two ways of animating something, dragging via a gesture, and hitting a reset button. The animation in my real app is complicated and requires logic for certain progress values, so I use your animation engine to animate certain things. In the demo, the logic is simply to decide which side of a card should be visible, and when it should flip over.

The slider represents the gesture progress, the reset button animates the gesture back to 0, and the plus button instantly changes the gesture progress to 1 (so you can mash Plus and Reset and the error is more likely to happen).

https://github.com/Naxum/INTUAnimationError

[smileyborg]

Your example project is awesome, thank you very much for sharing -- I can easily reproduce the issue, and see the exception:

*** Terminating app due to uncaught exception 'NSGenericException', reason: '*** Collection <__NSDictionaryM: 0x7f92c36032a0> was mutated while being enumerated.'

I agree that your fix would address this well. However the root cause of this issue is really excessive sharing of mutable state (dictionary). I think an even better way to solve this is to make the activeAnimations property of type NSDictionary, and when we need to mutate it, we'll make a copy first, then make the change, and then just assign the new version.

Let me make this change right now.

[smileyborg]

OK, this should be resolved by a0e31fd. Please take a look and let me know what you think.

Also, if you could give this change a try in your app just to confirm the crash is no longer reproducible, that would be great!

Thanks again for bringing this up and taking the time to put the sample project together!

[smileyborg]

I made a related change in 95e237b that is made possible by this change. Basically we can just clean up some code that is no longer needed. Please do test with that change as well!

[smileyborg]

@Naxum Did you get a chance to test this out? I'd like to push out a pod version once you confirm it's all good.

[Naxum]

Oh boy, I was heads down for a while. I'm checking it out now!

[Naxum]

@smileyborg Nice work! It seems resolved to me! :D

[smileyborg]

Awesome, I'll push out an update right now!