NSMutableArray modified while enumerating #39

Closed
notsoux opened this Issue Nov 14, 2012 · 5 comments

Comments

Projects
None yet
4 participants

notsoux commented Nov 14, 2012

Hi,
I'm using Mixpale sdk for a few days and I got the following error:
* Collection <__NSArrayM: xxxxxxxx> was mutated while being enumerated.*

I've inspected memory xxxxxxxx and it was the array used to collect events to be tracked (property called eventsQueue).

I got a read at the Mixpanel.m class and found that you've used @synchronized code when accessing eventsQueue object.

Our app is sending a lot of tracking events, even when no connection is available, so they can be sent as soon as the connection restart working again.

I noticed that if events were not sent because of connection lack, they will be archived using method archiveEvents, where's the exception seems to occour, I think because, at the same time, I'll continue adding events .

I added a @synchronized block around the code and problem seems disappear (done this also for archivePeople method).

- (void)archiveEvents
{
    @synchronized( self)  <-- added code
    {
        NSString *filePath = [self eventsFilePath];
        DevLog(@"%@ archiving events data to %@: %@", self, filePath, self.eventsQueue);
        if (![NSKeyedArchiver archiveRootObject:self.eventsQueue toFile:filePath]) {
            NSLog(@"%@ unable to archive events data", self);
        }
    }
}

I'd like to know if someone got into the same problem and an opinion if the solution can be viable.

Thanks!

William Pompei

We have the same problem. I don't know if the solution is viable, but it looks like it might be.

zadr commented Nov 27, 2012

I've run into this as well. The basic problem is that the Mixpanel library isn't particularly threadsafe.

In my case, it was pretty easy to ensure that my Mixpanel object is never accessed from a thread other than the one it was created on. While that doesn't fix the problem, it does avoid it pretty nicely.

Contributor

neilrahilly commented Dec 6, 2012

@zadr let me know of specific instances and i'm happy to fix them

zadr commented Dec 6, 2012

I think you fixed the cases that I was running into with the @synchronized calls, and other ones that look problematic. My only other thought is that @synchronzied(self) can be kind of slow (compared to NSLock, pthread mutexes or dispatch_sync'ing to ensure serial execution) if you have a lot of events being tracked, but, I suspect thats a non-issue for a vast, vast majority of the people and probably not worth caring about.

Contributor

neilrahilly commented Dec 7, 2012

Thanks Zach. Agreed.

I've been meaning to give some thought too to leaning on NSOperation to
synchronize in a manner that's little simpler and more reliable. In the
meantime, I hope the @synchronized's will fix the present design.

Please keep the feedback coming. It's appreciated.

On Thu, Dec 6, 2012 at 2:21 PM, Zach Drayer notifications@github.comwrote:

I think you fixed the cases that I was running into with the @synchronizedhttps://github.com/synchronizedcalls, and other ones that look problematic. My only other thought is that
@synchronzied(self) can be kind of slow (compared to NSLock, pthread
mutexes or dispatch_sync'ing to ensure serial execution) if you have a lot
of events being tracked, but, I suspect thats a non-issue for a vast, vast
majority of the people and probably not worth caring about.


Reply to this email directly or view it on GitHubhttps://github.com/mixpanel/mixpanel-iphone/issues/39#issuecomment-11108719.

@neilrahilly neilrahilly added a commit that referenced this issue Jun 27, 2014

@neilrahilly neilrahilly increase synchronization
closes #39
ab19fbc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment