Concurrency issue #79

mrwanny · 2013-03-13T16:51:07Z

saving nano-bag from different threads can cause nano-store to crash
Below I wrote a test that cause the crash pretty consistently

//
// NanoStoreConcurrentTests.m
// NanoStore
//
//

import "NanoStore.h"

import "NanoStoreTests.h"

import "NSFNanoStore_Private.h"

import "NSFNanoGlobals_Private.h"

import "NanoStoreConcurrentTests.h"

define NDPERBRESOURCEBAG @"aBag"

@interface NanoStoreConcurrentTests ()

@Property (nonatomic,strong) NSOperationQueue *queue;
@Property (nonatomic,strong) NSFNanoBag *bag;

@EnD

@implementation NanoStoreConcurrentTests

(void)setUp
{
[super setUp];

_defaultTestInfo = [NSFNanoStore _defaultTestData];

[self setQueue:[[NSOperationQueue alloc] init]];
[self.queue setMaxConcurrentOperationCount:10];

NSFSetIsDebugOn (NO);
}
(void)tearDown
{

NSFSetIsDebugOn (NO);

[super tearDown];
}

pragma mark -

(void)testConcurrent
{
NSLog(@"##########################");
NSString *docs = [NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES) lastObject];
NSString *path = [docs stringByAppendingPathComponent:[NSString stringWithFormat:@"%@.sqlite",@"concurrentdb1"]];
NSError *erroro = nil;
NSFNanoStore *nanoStore = [NSFNanoStore createAndOpenStoreWithType:NSFPersistentStoreType path:path error:&erroro];
if (erroro) {
NSLog(@"error %@",[erroro localizedDescription]);
}
STAssertTrue (erroro == nil, @"store should be ok");
[nanoStore setSaveInterval:500];

NSString *bagName = [NSString stringWithFormat:@"%@-%@",NDPERBRESOURCEBAG,@"of_things"];
_bag = [nanoStore bagWithName:bagName];
NSError *error = nil;
[nanoStore addObject:[NSFNanoBag bagWithName:bagName] error:&error];
STAssertTrue (error == nil, @"Expected bag to be ok");

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
NSDictionary *tmp = @{@"things": @[@{@"id": @1, @"name": @"tom1"},
@{@"id": @2, @"name": @"tom2"},
@{@"id": @3, @"name": @"tom3"},
@{@"id": @4, @"name": @"tom4"},
@{@"id": @5, @"name": @"tom5"},
@{@"id": @6, @"name": @"tom6"},
@{@"id": @7, @"name": @"tom7"},
@{@"id": @8, @"name": @"tom8"},
@{@"id": @9, @"name": @"tom9"},
@{@"id": @10, @"name": @"tom10"},
@{@"id": @11, @"name": @"tom11"},
@{@"id": @12, @"name": @"tom12"},
@{@"id": @13, @"name": @"tom13"},
@{@"id": @14, @"name": @"tom14"},
@{@"id": @15, @"name": @"tom15"},
@{@"id": @16, @"name": @"tom16"},
@{@"id": @17, @"name": @"tom17"},
@{@"id": @18, @"name": @"tom18"}]};
[tmp enumerateKeysAndObjectsWithOptions:NSEnumerationConcurrent usingBlock:^(id key, id obj, BOOL *stop) {
if ([key isEqualToString:@"things"]) {
for (id anObj in obj) {
```
            NSFNanoObject *nanoObject = nil;

            NSFNanoSearch *search = [NSFNanoSearch searchWithStore:nanoStore];
            search.attribute = [NSString stringWithFormat:@"%@.%@",@"things",@"id"];
            search.match = NSFEqualTo;
            search.value = [NSString stringWithFormat:@"%@",[anObj objectForKey:@"id"]];
            search.bag = self.bag;

            NSError *searchError = nil;
            id results = [search searchObjectsWithReturnType:NSFReturnObjects error:&searchError];

            if ([[results allKeys] count] > 0) {

                STAssertTrue ([[results allKeys] count] == 1, @"Expected bag only 1 result");
                for (NSString *aKey in [results allKeys]) {
                    nanoObject = [results objectForKey:aKey];
                }
                [nanoObject addEntriesFromDictionary:anObj];

            }else {
                nanoObject = [NSFNanoObject nanoObjectWithDictionary:anObj];
            }

            NSError *saveError = nil;

            [self.bag addObject:nanoObject error:&saveError];
            STAssertTrue (saveError == nil, @"Expected saveError to be ok");


        }
    }
}];

NSError *saveError = nil;
[self.bag saveAndReturnError:&saveError];
STAssertTrue (saveError == nil, @"Expected saveError to be ok");
```
});

void (^ablock)() = ^() {
NSMutableArray *array = [NSMutableArray arrayWithCapacity:1000];
```
for (int i =100; i<1000; i++) {
    [array addObject:@{@"id":@(i),@"name": [NSString stringWithFormat:@"bob-%d",i]}];
}
[@{@"things": array} enumerateKeysAndObjectsWithOptions:NSEnumerationConcurrent usingBlock:^(id key, id obj, BOOL *stop) {
    if ([key isEqualToString:@"things"]) {
        for (id anObj in obj) {

            NSFNanoObject *nanoObject = nil;

            NSFNanoSearch *search = [NSFNanoSearch searchWithStore:nanoStore];
            search.attribute = [NSString stringWithFormat:@"%@.%@",@"things",@"id"];
            search.match = NSFEqualTo;
            search.value = [NSString stringWithFormat:@"%@",[anObj objectForKey:@"id"]];
            search.bag = self.bag;

            NSError *searchError = nil;
            id results = [search searchObjectsWithReturnType:NSFReturnObjects error:&searchError];

            if ([[results allKeys] count] > 0) {

                STAssertTrue ([[results allKeys] count] == 1, @"Expected bag only 1 result");
                for (NSString *aKey in [results allKeys]) {
                    nanoObject = [results objectForKey:aKey];
                }
                [nanoObject addEntriesFromDictionary:anObj];

            }else {
                nanoObject = [NSFNanoObject nanoObjectWithDictionary:anObj];
            }

            NSError *saveError = nil;

            [self.bag addObject:nanoObject error:&saveError];
            STAssertTrue (saveError == nil, @"Expected saveError to be ok");


        }
        NSError *bagError = nil;
        [self.bag saveAndReturnError:&bagError];
        STAssertTrue (bagError == nil, @"Expected bagError to be ok");
    }
}];

NSError *saveError = nil;
[self.bag saveAndReturnError:&saveError];
STAssertTrue (saveError == nil, @"Expected saveError to be ok");
```
};
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ablock);

}

@EnD

The text was updated successfully, but these errors were encountered:

tciuro · 2013-03-13T19:47:35Z

Wanny, NanoStore was never tested in multithreaded mode. However, not all is lost. Before anything else, SQLite has rules that must be followed:

Make sure you're compiling SQLite with -DTHREADSAFE=1.
Make sure that each thread/block/operation opens the database file and keeps its own sqlite structure.

Looks like the store is being shared across the dispatched blocks, which is a big no-no in SQLite:

NSFNanoSearch *search = [NSFNanoSearch searchWithStore:nanoStore];

tciuro · 2013-03-13T19:56:47Z

More here:

SQLite And Multiple Threads
http://www.sqlite.org/threadsafe.html

mrwanny · 2013-03-14T04:35:03Z

Thanks a lot for the suggestion! You put me on the right direction.

tciuro · 2013-04-10T21:41:05Z

Any news on this? ;-)

tciuro · 2013-05-03T16:21:34Z

Wanny, any luck? Can we close this one?

kareemk · 2013-05-13T15:08:27Z

I may be having issues with this and it seems like per https://github.com/tciuro/NanoStore/blob/master/Classes/Advanced/NSFNanoEngine.m#L148 that all connections should be safe as they're opened in SQLITE_OPEN_FULLMUTEX mode (assuming that sqllite has been compiled with multi-threading support). So is a separate connection really necessary per thread/block/operation?

tciuro · 2013-05-21T03:02:24Z

Well, multithreaded support has been incrementally added and fixed over the years. To tell you the truth, I don't know where we stand. I just read the following post:

http://dev.yorhel.nl/doc/sqlaccess

I'd like to find out whether this is correct or not. I'm hesitant to add support if it's not reliable.

billgarrison · 2013-05-22T04:28:24Z

I read that article at some point too. From what I can tell, a SQLite3 database connection configured in full mutex mode will pretty serialize access as need from every direction: multiple threads within the same process, multiple processes touching the same database file.

For dealing with the documented issue of database connection error information not being safe from concurrent mutation, I've implemented the recommended strategy of locking the database while asking for the last recorded error code and diagnostic message. Here's a C function that returns the SQLite3 error status as an NSError that should be reliable:

NSString *SQLiteErrorDomain = @"SQLiteErrorDomain";
NSError *SQLiteError(sqlite3 *db) {
    NSError *error = nil;
    
    if (db) {
        
        /* Lock the database connection to prevent other activity from disturbing the currently recorded internal error code and message. 
         */
        
        sqlite3_mutex *mutex = sqlite3_mutex_alloc(SQLITE_MUTEX_FAST);
        if (mutex) {
            sqlite3_mutex_enter(mutex);
            
            int lastErrorCode = sqlite3_errcode(db);
            const char *lastErrorMessage = sqlite3_errmsg(db);
            
            error = [NSError errorWithDomain:SQLiteErrorDomain code:lastErrorCode userInfo:
                     @{NSLocalizedDescriptionKey : [NSString stringWithUTF8String:lastErrorMessage]}
                     ];
            
            sqlite3_mutex_leave(mutex);
            sqlite3_mutex_free(mutex);
            mutex = NULL;
        }
    }
    
    return error;
}

YapDatabase https://github.com/yaptv/YapDatabase takes an interesting approach with SQLite3 concurrency. It totally disables internal SQLite3 locking and manages concurrent access to a database connection through dedicated GCD queues. I did look through this code a bit, and it goes to great lengths to provide reasonable concurrent access: multiple reads are not blocked, writes block as minimally as possible. Pretty the same behavior as provided natively by the SQLite3 library, but using GCD queues.

I'm not sure how YapDatabase using sqlite3 in no-lock mode compares with NanoStore using sqlite3 in fullmutex mode. That could be a lot of apples vs. oranges vs carrots. :-) SQLite3 with no internal locks is supposed to be a lot faster than full locks. But the complexity of logic for managing concurrent db access just got pushed over to YapDatabase and GCD queues. An actual performance measurement would be interesting to see.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Concurrency issue #79

Concurrency issue #79

mrwanny commented Mar 13, 2013

tciuro commented Mar 13, 2013

tciuro commented Mar 13, 2013

mrwanny commented Mar 14, 2013

tciuro commented Apr 10, 2013

tciuro commented May 3, 2013

kareemk commented May 13, 2013

tciuro commented May 21, 2013

billgarrison commented May 22, 2013

Concurrency issue #79

Concurrency issue #79

Comments

mrwanny commented Mar 13, 2013

import "NanoStore.h"

import "NanoStoreTests.h"

import "NSFNanoStore_Private.h"

import "NSFNanoGlobals_Private.h"

import "NanoStoreConcurrentTests.h"

define NDPERBRESOURCEBAG @"aBag"

pragma mark -

tciuro commented Mar 13, 2013

tciuro commented Mar 13, 2013

mrwanny commented Mar 14, 2013

tciuro commented Apr 10, 2013

tciuro commented May 3, 2013

kareemk commented May 13, 2013

tciuro commented May 21, 2013

billgarrison commented May 22, 2013