Collection was mutated while being enumerated- archiving data and writing to file with NSCoder

Question

In my app I am periodically writing a set of dynamic data to file. The data object gets updated about every second. Occasionally I get a "Collection was mutated while being mutated" exception on one of the lines in my encodeWithCoder: method. Each object is encoded like this:

[aCoder encodeObject:self.speeds forKey:@"speeds"];

Where self.speeds is an NSMutableArray. I assume the problem is that the data is being updated while it is being encoded. I tried using @synchronize in the encoding and saving blocks and I also tried making the property atomic as opposed to nonatomic, but neither worked. The saving is happening in the background. Any ideas of how to save this data in the background while it is being updated? I feel like making a copy and then saving the copy would work, but wouldn't the same problem arise? Thanks!

---

Edit 1:

The idea in the app is that I open a map view, which periodically updates a singleton class with contains an array of data objects, with each data object being a user's map info. In each data object, the user's locations, speeds, altitudes, distance, etc. Every three times the location manager updates the user's location, I update the current data object (the 'live' data object which was just created to track this trip- there can only be one 'live' data object at any time) with the new information.

I would like to write the entire singleton to a file every x minutes, but sometimes the writing and the update happen at the same time and I get this error (or at least that is what I assume is causing this crash). Is the problem here with my code or my design pattern?

---

This is the encoding method in my custom class:

- (void)encodeWithCoder:(NSCoder*)aCoder {
    @synchronized([SingletonDataController sharedSingleton]) {
        [aCoder encodeObject:[[lineLats copy] autorelease] forKey:@"lineLats"];
        [aCoder encodeObject:[[lineLongs copy] autorelease] forKey:@"lineLongs"];
        [aCoder encodeObject:[[horizontalAccuracies copy] autorelease] forKey:@"horAcc"];
        [aCoder encodeObject:[[verticalAccuracies copy] autorelease] forKey:@"vertAcc"];
        [aCoder encodeObject:[[speeds copy] autorelease] forKey:@"speeds"];
        [aCoder encodeObject:[[overlayColors copy] autorelease] forKey:@"colors"];
        [aCoder encodeObject:[[annotationLats copy] autorelease] forKey:@"annLats"];
        [aCoder encodeObject:[[annotationLongs copy] autorelease] forKey:@"annLongs"];
        [aCoder encodeObject:[[locationManagerStartDate copy] autorelease] forKey:@"startDate"];
        [aCoder encodeObject:[[locationManagerStartDateString copy] autorelease] forKey:@"locStartDateString"];
        [aCoder encodeObject:[[mapTitleString copy] autorelease] forKey:@"title"];
        [aCoder encodeObject:[[shortDateStringBackupCopy copy] autorelease] forKey:@"backupString"];

        [aCoder encodeFloat:pathDistance forKey:@"pathDistance"];
        [aCoder encodeFloat:linearDistance forKey:@"linearDistance"];
        [aCoder encodeFloat:altitudeChange forKey:@"altitudeChange"];
        [aCoder encodeFloat:averageSpeedWithFilter forKey:@"avWithFilter"];
        [aCoder encodeFloat:averageSpeedWithoutFilter forKey:@"avWithoutFilter"];

        [aCoder encodeInt:totalTripTimeInSeconds forKey:@"totalTimeInSecs"];
    }
}

This is the update method (there is more code in the method and other methods called in the update method, but I'm omitting everything that doesn't reference the 'live' dataObject object; the one being updated):

- (void)locationManager:(CLLocationManager*)manager didUpdateToLocation:(CLLocation*)newLocation fromLocation:(CLLocation*)oldLocation {
    @synchronized([SingletonDataController sharedSingleton]) {
        //create temporary location for last logged location
        CLLocation* lastLocation;
        if([dataObject.lineLats lastObject] && [dataObject.lineLongs lastObject]) {
            lastLocation = [[CLLocation alloc] initWithLatitude:[[dataObject.lineLats lastObject] floatValue] longitude:[[dataObject.lineLongs lastObject] floatValue]];
        } else {
            lastLocation = [oldLocation retain];
        }

        //.....

        //periodically add horizontal/vertical accuracy
        if(iterations > 0 && iterations % 4 == 0) {
            [dataObject.horizontalAccuracies addObject:[NSNumber numberWithFloat:[newLocation horizontalAccuracy]]];
            [dataObject.verticalAccuracies addObject:[NSNumber numberWithFloat:[newLocation verticalAccuracy]]];
        }

        //.....

        //accumulate some speed data
        if(iterations % 2 == 0) {
            NSNumber* speedNum = [[NSNumber alloc] initWithFloat:[newLocation speed]];
            [dataObject.speeds addObject:speedNum];
            [speedNum release];
        }

        //.....

        //add latitude and longitude
        NSNumber* lat = [[NSNumber alloc] initWithFloat:[newLocation coordinate].latitude];
        NSNumber* lon = [[NSNumber alloc] initWithFloat:[newLocation coordinate].longitude];
        if(fabs([lat floatValue]) > .0001 && fabs([lon floatValue]) > .0001) {
            [dataObject.lineLats addObject:lat];
            [dataObject.lineLongs addObject:lon];
        }

        if(iterations % 60 == 0) {
            [[SingletonDataController sharedSingleton] synchronize];
        }
    }
}

And finally the synchronize method in the SingletonDataController class (updated so that now the synchronization occurs within the asynchronous block as per Tommy's answer):

dispatch_async(self.backgroundQueue, ^{
    @synchronized([SingletonDataController sharedSingleton]) {
        NSLog(@"sync");
        NSData* singletonData = [NSKeyedArchiver archivedDataWithRootObject:
            [SingletonDataController sharedSingleton]];

        if(!singletonData) {
            return;
        }

        NSString* filePath = [SingletonDataController getDataFilePath];
        [singletonData writeToFile:filePath atomically:YES];
    }
});

where backgroundQueue is created like this:

[sharedSingleton setBackgroundQueue:dispatch_queue_create("com.xxxx.xx", NULL)];

I can post more code if needed, but those seem to be the important parts.

Answer 1

You perform a dispatch_async within one of your @synchronizes. The stuff in there isn't subject to the implicit lock built into the synchronise; all that happens it that you get the lock, dispatch the block and then release the lock. So the block can easily happen outside of the lock (and, indeed, you would expect that usually it will).

To stick with the synchronisation path, you want to @synchronize within the block rather than outside it. However you might try to come up with a less forceful approach, such as performing all your updates on a single serial dispatch queue and allowing them to push relevant new values onto the main queue.