From 975750360bc39f4fb1e528cd966a515f228a3f0f Mon Sep 17 00:00:00 2001 From: Richard Ross Date: Wed, 4 Nov 2015 16:46:00 -0800 Subject: [PATCH] Made all accesses to PFSQLiteDatabaseResult & Statement thread-safe. I think this is what was causing the crashes seen in #290, but I'm not 100% sure. At the very least, accessing them this way was in violation of the SQLite spec. --- .../LocalDataStore/SQLite/PFSQLiteDatabase.m | 17 ++- .../SQLite/PFSQLiteDatabaseResult.h | 2 +- .../SQLite/PFSQLiteDatabaseResult.m | 107 +++++++++++------- .../LocalDataStore/SQLite/PFSQLiteStatement.h | 5 +- .../LocalDataStore/SQLite/PFSQLiteStatement.m | 33 +++--- Parse/Internal/ThreadSafety/PFThreadsafety.h | 8 ++ 6 files changed, 110 insertions(+), 62 deletions(-) diff --git a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabase.m b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabase.m index 41cfe35cf..fb9c07786 100644 --- a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabase.m +++ b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabase.m @@ -23,6 +23,7 @@ #import "PFSQLiteDatabaseResult.h" #import "PFSQLiteStatement.h" #import "Parse_Private.h" +#import "PFThreadsafety.h" static NSString *const PFSQLiteDatabaseBeginExclusiveOperationCommand = @"BEGIN EXCLUSIVE"; static NSString *const PFSQLiteDatabaseCommitOperationCommand = @"COMMIT"; @@ -67,8 +68,16 @@ - (instancetype)initWithPath:(NSString *)path { _databaseClosedTaskCompletionSource = [[BFTaskCompletionSource alloc] init]; _databasePath = [path copy]; - _databaseQueue = dispatch_queue_create("com.parse.sqlite.db.queue", DISPATCH_QUEUE_SERIAL); - _databaseExecutor = [BFExecutor executorWithDispatchQueue:_databaseQueue]; + + _databaseQueue = PFThreadsafetyCreateQueueForObject(self); + _databaseExecutor = [BFExecutor executorWithBlock:^(dispatch_block_t block) { + // Execute asynchrounously on the proper queue. + // Seems a bit backwards, but we don't have PFThreadsafetySafeDispatchAsync. + dispatch_async(dispatch_get_global_queue(0, 0), ^{ + PFThreadsafetySafeDispatchSync(_databaseQueue, block); + }); + }]; + _cachedStatements = [[NSMutableDictionary alloc] init]; return self; @@ -178,7 +187,7 @@ - (BFTask *)_executeQueryAsync:(NSString *)sql withArgumentsInArray:(NSArray *)a sqlite3_finalize(sqliteStatement); return [BFTask taskWithError:[self _errorWithErrorCode:resultCode]]; } - statement = [[PFSQLiteStatement alloc] initWithStatement:sqliteStatement]; + statement = [[PFSQLiteStatement alloc] initWithStatement:sqliteStatement queue:_databaseQueue]; if (enableCaching) { [self _cacheStatement:statement forQuery:sql]; @@ -206,7 +215,7 @@ - (BFTask *)_executeQueryAsync:(NSString *)sql withArgumentsInArray:(NSArray *)a [self _bindObject:args[idx] toColumn:(idx + 1) inStatement:statement]; } - PFSQLiteDatabaseResult *result = [[PFSQLiteDatabaseResult alloc] initWithStatement:statement]; + PFSQLiteDatabaseResult *result = [[PFSQLiteDatabaseResult alloc] initWithStatement:statement queue:_databaseQueue]; return [BFTask taskWithResult:result]; } diff --git a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.h b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.h index d63d36f62..97b54b9f1 100644 --- a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.h +++ b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.h @@ -15,7 +15,7 @@ NS_ASSUME_NONNULL_BEGIN @interface PFSQLiteDatabaseResult : NSObject -- (instancetype)initWithStatement:(PFSQLiteStatement *)statement; +- (instancetype)initWithStatement:(PFSQLiteStatement *)statement queue:(dispatch_queue_t)queue; /*! Move current result to next row. Returns true if next result exists. False if current result diff --git a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.m b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.m index cdbeb0e89..dd371751d 100644 --- a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.m +++ b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteDatabaseResult.m @@ -12,11 +12,13 @@ #import #import "PFSQLiteStatement.h" +#import "PFThreadsafety.h" @interface PFSQLiteDatabaseResult () @property (nonatomic, copy, readonly) NSDictionary *columnNameToIndexMap; @property (nonatomic, strong, readonly) PFSQLiteStatement *statement; +@property (nonatomic, strong, readonly) dispatch_queue_t databaseQueue; @end @@ -24,9 +26,10 @@ @implementation PFSQLiteDatabaseResult @synthesize columnNameToIndexMap = _columnNameToIndexMap; -- (instancetype)initWithStatement:(PFSQLiteStatement *)stmt { +- (instancetype)initWithStatement:(PFSQLiteStatement *)stmt queue:(dispatch_queue_t)queue { if ((self = [super init])) { _statement = stmt; + _databaseQueue = queue; } return self; } @@ -36,7 +39,9 @@ - (BOOL)next { } - (int)step { - return sqlite3_step([self.statement sqliteStatement]); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return sqlite3_step([self.statement sqliteStatement]); + }); } - (BOOL)close { @@ -48,7 +53,9 @@ - (int)intForColumn:(NSString *)columnName { } - (int)intForColumnIndex:(int)columnIndex { - return sqlite3_column_int([self.statement sqliteStatement], columnIndex); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return sqlite3_column_int([self.statement sqliteStatement], columnIndex); + }); } - (long)longForColumn:(NSString *)columnName { @@ -56,7 +63,9 @@ - (long)longForColumn:(NSString *)columnName { } - (long)longForColumnIndex:(int)columnIndex { - return (long)sqlite3_column_int64([self.statement sqliteStatement], columnIndex); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return (long)sqlite3_column_int64([self.statement sqliteStatement], columnIndex); + }); } - (BOOL)boolForColumn:(NSString *)columnName { @@ -64,7 +73,9 @@ - (BOOL)boolForColumn:(NSString *)columnName { } - (BOOL)boolForColumnIndex:(int)columnIndex { - return ([self intForColumnIndex:columnIndex] != 0); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return ([self intForColumnIndex:columnIndex] != 0); + }); } - (double)doubleForColumn:(NSString *)columnName { @@ -72,7 +83,9 @@ - (double)doubleForColumn:(NSString *)columnName { } - (double)doubleForColumnIndex:(int)columnIndex { - return sqlite3_column_double([self.statement sqliteStatement], columnIndex); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return sqlite3_column_double([self.statement sqliteStatement], columnIndex); + }); } - (NSString *)stringForColumn:(NSString *)columnName { @@ -80,15 +93,17 @@ - (NSString *)stringForColumn:(NSString *)columnName { } - (NSString *)stringForColumnIndex:(int)columnIndex { - if ([self columnIndexIsNull:columnIndex]) { - return nil; - } + return PFThreadSafetyPerform(_databaseQueue, ^NSString *{ + if ([self columnIndexIsNull:columnIndex]) { + return nil; + } - const char *str = (const char *)sqlite3_column_text([self.statement sqliteStatement], columnIndex); - if (!str) { - return nil; - } - return [NSString stringWithUTF8String:str]; + const char *str = (const char *)sqlite3_column_text([self.statement sqliteStatement], columnIndex); + if (!str) { + return nil; + } + return [NSString stringWithUTF8String:str]; + }); } - (NSDate *)dateForColumn:(NSString *)columnName { @@ -105,16 +120,18 @@ - (NSData *)dataForColumn:(NSString *)columnName { } - (NSData *)dataForColumnIndex:(int)columnIndex { - if ([self columnIndexIsNull:columnIndex]) { - return nil; - } + return PFThreadSafetyPerform(_databaseQueue, ^NSData *{ + if ([self columnIndexIsNull:columnIndex]) { + return nil; + } - int size = sqlite3_column_bytes([self.statement sqliteStatement], columnIndex); - const char *buffer = sqlite3_column_blob([self.statement sqliteStatement], columnIndex); - if (buffer == nil) { - return nil; - } - return [NSData dataWithBytes:buffer length:size]; + int size = sqlite3_column_bytes([self.statement sqliteStatement], columnIndex); + const char *buffer = sqlite3_column_blob([self.statement sqliteStatement], columnIndex); + if (buffer == nil) { + return nil; + } + return [NSData dataWithBytes:buffer length:size]; + }); } - (id)objectForColumn:(NSString *)columnName { @@ -122,17 +139,19 @@ - (id)objectForColumn:(NSString *)columnName { } - (id)objectForColumnIndex:(int)columnIndex { - int columnType = sqlite3_column_type([self.statement sqliteStatement], columnIndex); - switch (columnType) { - case SQLITE_INTEGER: - return @([self longForColumnIndex:columnIndex]); - case SQLITE_FLOAT: - return @([self doubleForColumnIndex:columnIndex]); - case SQLITE_BLOB: - return [self dataForColumnIndex:columnIndex]; - default: - return [self stringForColumnIndex:columnIndex]; - } + return PFThreadSafetyPerform(_databaseQueue, ^id{ + int columnType = sqlite3_column_type([self.statement sqliteStatement], columnIndex); + switch (columnType) { + case SQLITE_INTEGER: + return @([self longForColumnIndex:columnIndex]); + case SQLITE_FLOAT: + return @([self doubleForColumnIndex:columnIndex]); + case SQLITE_BLOB: + return [self dataForColumnIndex:columnIndex]; + default: + return [self stringForColumnIndex:columnIndex]; + } + }); } - (BOOL)columnIsNull:(NSString *)columnName { @@ -140,7 +159,9 @@ - (BOOL)columnIsNull:(NSString *)columnName { } - (BOOL)columnIndexIsNull:(int)columnIndex { - return (sqlite3_column_type([self.statement sqliteStatement], columnIndex) == SQLITE_NULL); + return PFThreadSafetyPerform(_databaseQueue, ^{ + return (sqlite3_column_type([self.statement sqliteStatement], columnIndex) == SQLITE_NULL); + }); } - (int)columnIndexForName:(NSString *)columnName { @@ -154,13 +175,15 @@ - (int)columnIndexForName:(NSString *)columnName { - (NSDictionary *)columnNameToIndexMap { if (!_columnNameToIndexMap) { - int columnCount = sqlite3_column_count([self.statement sqliteStatement]); - NSMutableDictionary *mutableColumnNameToIndexMap = [[NSMutableDictionary alloc] initWithCapacity:columnCount]; - for (int i = 0; i < columnCount; ++i) { - NSString *key = [NSString stringWithUTF8String:sqlite3_column_name([self.statement sqliteStatement], i)]; - mutableColumnNameToIndexMap[[key lowercaseString]] = @(i); - } - _columnNameToIndexMap = mutableColumnNameToIndexMap; + PFThreadsafetySafeDispatchSync(_databaseQueue, ^{ + int columnCount = sqlite3_column_count([self.statement sqliteStatement]); + NSMutableDictionary *mutableColumnNameToIndexMap = [[NSMutableDictionary alloc] initWithCapacity:columnCount]; + for (int i = 0; i < columnCount; ++i) { + NSString *key = [NSString stringWithUTF8String:sqlite3_column_name([self.statement sqliteStatement], i)]; + mutableColumnNameToIndexMap[[key lowercaseString]] = @(i); + } + _columnNameToIndexMap = mutableColumnNameToIndexMap; + }); } return _columnNameToIndexMap; } diff --git a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.h b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.h index 04e181c43..148ec156f 100644 --- a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.h +++ b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.h @@ -18,9 +18,10 @@ typedef struct sqlite3_stmt sqlite3_stmt; @interface PFSQLiteStatement : NSObject -@property (atomic, assign, readonly) sqlite3_stmt *sqliteStatement; +@property (nonatomic, assign, readonly) sqlite3_stmt *sqliteStatement; +@property (nonatomic, strong, readonly) dispatch_queue_t databaseQueue; -- (instancetype)initWithStatement:(sqlite3_stmt *)stmt; +- (instancetype)initWithStatement:(sqlite3_stmt *)stmt queue:(dispatch_queue_t)databaseQueue; - (BOOL)close; - (BOOL)reset; diff --git a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.m b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.m index 03f31b6ff..b25014678 100644 --- a/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.m +++ b/Parse/Internal/LocalDataStore/SQLite/PFSQLiteStatement.m @@ -11,13 +11,16 @@ #import +#import "PFThreadsafety.h" + @implementation PFSQLiteStatement -- (instancetype)initWithStatement:(sqlite3_stmt *)stmt { +- (instancetype)initWithStatement:(sqlite3_stmt *)stmt queue:(dispatch_queue_t)databaseQueue { self = [super init]; if (!stmt || !self) return nil; _sqliteStatement = stmt; + _databaseQueue = databaseQueue; return self; } @@ -27,23 +30,27 @@ - (void)dealloc { } - (BOOL)close { - if (!_sqliteStatement) { - return YES; - } + return PFThreadSafetyPerform(_databaseQueue, ^BOOL{ + if (!_sqliteStatement) { + return YES; + } - int resultCode = sqlite3_finalize(_sqliteStatement); - _sqliteStatement = nil; + int resultCode = sqlite3_finalize(_sqliteStatement); + _sqliteStatement = nil; - return (resultCode == SQLITE_OK || resultCode == SQLITE_DONE); + return (resultCode == SQLITE_OK || resultCode == SQLITE_DONE); + }); } - (BOOL)reset { - if (!_sqliteStatement) { - return YES; - } - - int resultCode = sqlite3_reset(_sqliteStatement); - return (resultCode == SQLITE_OK || resultCode == SQLITE_DONE); + return PFThreadSafetyPerform(_databaseQueue, ^BOOL{ + if (!_sqliteStatement) { + return YES; + } + + int resultCode = sqlite3_reset(_sqliteStatement); + return (resultCode == SQLITE_OK || resultCode == SQLITE_DONE); + }); } @end diff --git a/Parse/Internal/ThreadSafety/PFThreadsafety.h b/Parse/Internal/ThreadSafety/PFThreadsafety.h index 7ca2a64c0..d8d9c1cdb 100644 --- a/Parse/Internal/ThreadSafety/PFThreadsafety.h +++ b/Parse/Internal/ThreadSafety/PFThreadsafety.h @@ -11,3 +11,11 @@ extern dispatch_queue_t PFThreadsafetyCreateQueueForObject(id object); extern void PFThreadsafetySafeDispatchSync(dispatch_queue_t queue, dispatch_block_t block); + + +// PFThreadsafetySafeDispatchSync, but with a return type. +#define PFThreadSafetyPerform(queue, block) ({ \ + __block typeof((block())) result; \ + PFThreadsafetySafeDispatchSync(queue, ^{ result = block(); }); \ + result; \ +})