search

tciuro / NanoStore

NanoStore is an open source, lightweight schema-less local key-value document store written in Objective-C for Mac OS X and iOS.
Other
404 stars 39 forks source link

Concurrency issue #79

Open mrwanny opened 11 years ago

mrwanny commented 11 years ago

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

}

@end

tciuro commented 11 years ago

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

1) Make sure you're compiling SQLite with -DTHREADSAFE=1. 2) 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 commented 11 years ago

More here:

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

mrwanny commented 11 years ago

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

tciuro commented 11 years ago

Any news on this? ;-)

tciuro commented 11 years ago

Wanny, any luck? Can we close this one?

kareemk commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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.