Collection mutation exception in -currentConsoleLogWithDateStamps:

[epatey]

Thanks for the code!

-currentConsoleLogWithDateStamps: accesses self.consoleMessages from the wrong queue. Because of this, while enumerating the collection here:

for (BSKLogMessage *msg in self.consoleMessages) {

concurrent calls to +addLogMessage: mutate the collection causing an exception.

This could mostly be fixed with [self.consoleMessages copy]. Technically, though, since it's never good to access an NS collection from multiple threads a better fix probably requires a dispatch_sync(self.logQueue, ^{. If it's cool to be on that queue for a while, just execute the whole loop on the queue.

- (NSString *)currentConsoleLogWithDateStamps:(BOOL)dateStamps
{
    NSMutableString *string = [NSMutableString string];

    dispatch_sync(self.logQueue, ^{
        char fdate[24];
        for (BSKLogMessage *msg in self.consoleMessages) {
            if (dateStamps) {
                time_t timestamp = (time_t) msg.timestamp;
                struct tm *lt = localtime(&timestamp);
                strftime(fdate, 24, "%Y-%m-%d %T", lt);
                [string appendFormat:@"%s.%03d %@\n", fdate, (int) (1000.0 * (msg.timestamp - floor(msg.timestamp))), msg.message];
            } else {
                [string appendFormat:@"%@\n", msg.message];
            }
        }
    });

    return string;
}

Alternatively, you could just copy the collection while dispatched to the proper queue:

- (NSString *)currentConsoleLogWithDateStamps:(BOOL)dateStamps
{
    NSMutableString *string = [NSMutableString string];

    __block NSArray *arrayToEnumerate;
    dispatch_sync(self.logQueue, ^{
        arrayToEnumerate = [self.consoleMessages copy];
    });

    char fdate[24];
    for (BSKLogMessage *msg in arrayToEnumerate) {
        if (dateStamps) {
            time_t timestamp = (time_t) msg.timestamp;
            struct tm *lt = localtime(&timestamp);
            strftime(fdate, 24, "%Y-%m-%d %T", lt);
            [string appendFormat:@"%s.%03d %@\n", fdate, (int) (1000.0 * (msg.timestamp - floor(msg.timestamp))), msg.message];
        } else {
            [string appendFormat:@"%@\n", msg.message];
        }
    }

    return string;
}

I'll happily do a pull request with either approach based on feedback.

[marcoarment]

Good catch.

I'm a little worried about dispatch_syncing from this, since it's called from the main thread and the log queue might back up under heavy logging (many messages per second).

An alternative might be to dispatch all self.consoleMessages operations onto the main queue. The downside may be clogging up the main thread under heavy logging, but it might be worth it for faster safe access to the messages under normal circumstances.

What do you think?

[epatey]

I'm not a fan of doing logging work on the main queue. Good point about blocking main thread if there's a lot of backed up logging.

Better still might be to just make currentConsoleLogWithDateStamps async with a completion block and then just dispatch-async, but that may be more involved.

How about this? #19

[marcoarment]

Looks good. Thanks.