From 47258b02b179b212c7c031daa8b70fc2563509cd Mon Sep 17 00:00:00 2001 From: Christopher Snowhill Date: Fri, 15 Jul 2022 07:00:11 -0700 Subject: [PATCH] [Core Data] Add access locking Apparently we need this to prevent Core Data from stomping on itself when another thread accesses it. Signed-off-by: Christopher Snowhill --- Application/AppController.m | 6 +++++ Playlist/PlaylistController.h | 2 ++ Playlist/PlaylistController.m | 24 ++++++++++++++++++ Playlist/PlaylistEntry.m | 7 ++++++ Playlist/PlaylistLoader.m | 12 +++++++++ Preferences/Preferences/PathSuggester.m | 3 +++ .../SandboxPathBehaviorController.m | 18 +++++++++++++ Spotlight/SpotlightPlaylistEntry.m | 3 +++ Utils/SQLiteStore.m | 5 ++++ Utils/SandboxBroker.m | 25 ++++++++++++++++--- 10 files changed, 101 insertions(+), 4 deletions(-) diff --git a/Application/AppController.m b/Application/AppController.m index 9c884b9e9..0f6b44391 100644 --- a/Application/AppController.m +++ b/Application/AppController.m @@ -222,7 +222,9 @@ static AppController *kAppController = nil; request.predicate = predicate; NSError *error = nil; + [playlistController.persistentContainerLock lock]; NSArray *results = [playlistController.persistentContainer.viewContext executeFetchRequest:request error:&error]; + [playlistController.persistentContainerLock unlock]; if(results && [results count] == 1) { PlaylistEntry *pe = results[0]; @@ -428,7 +430,9 @@ static AppController *kAppController = nil; for(PlaylistEntry *pe in playlistController.arrangedObjects) { if(pe.deLeted) { + [playlistController.persistentContainerLock lock]; [moc deleteObject:pe]; + [playlistController.persistentContainerLock unlock]; continue; } if([artLeftovers objectForKey:pe.artHash]) { @@ -436,9 +440,11 @@ static AppController *kAppController = nil; } } + [playlistController.persistentContainerLock lock]; for(NSString *key in artLeftovers) { [moc deleteObject:[artLeftovers objectForKey:key]]; } + [playlistController.persistentContainerLock unlock]; [playlistController commitPersistentStore]; diff --git a/Playlist/PlaylistController.h b/Playlist/PlaylistController.h index 331d6914d..2076cab22 100644 --- a/Playlist/PlaylistController.h +++ b/Playlist/PlaylistController.h @@ -67,6 +67,7 @@ typedef NS_ENUM(NSInteger, URLOrigin) { @property(retain) NSString *_Nullable totalTime; @property(retain) NSString *_Nullable currentStatus; +@property(strong, nonatomic, readonly) NSLock *_Nonnull persistentContainerLock; @property(strong, nonatomic, readonly) NSPersistentContainer *_Nonnull persistentContainer; @property(strong, nonatomic, readonly) NSMutableDictionary *_Nonnull persistentArtStorage; @@ -141,6 +142,7 @@ typedef NS_ENUM(NSInteger, URLOrigin) { - (void)readShuffleListFromDataStore; + (NSPersistentContainer *_Nonnull)sharedPersistentContainer; ++ (NSLock *_Nonnull)sharedPersistentContainerLock; // reload metadata of selection - (IBAction)reloadTags:(id _Nullable)sender; diff --git a/Playlist/PlaylistController.m b/Playlist/PlaylistController.m index bb03f0d8c..805e73efa 100644 --- a/Playlist/PlaylistController.m +++ b/Playlist/PlaylistController.m @@ -30,6 +30,8 @@ extern BOOL kAppControllerShuttingDown; +NSLock *kPersistentContainerLock = nil; + NSPersistentContainer *kPersistentContainer = nil; @implementation PlaylistController @@ -124,6 +126,10 @@ static void *playlistControllerContext = &playlistControllerContext; }]; kPersistentContainer = self.persistentContainer; + + _persistentContainerLock = [[NSLock alloc] init]; + + kPersistentContainerLock = self.persistentContainerLock; self.persistentContainer.viewContext.mergePolicy = NSMergeByPropertyObjectTrumpMergePolicy; @@ -137,6 +143,10 @@ static void *playlistControllerContext = &playlistControllerContext; return kPersistentContainer; } ++ (NSLock *)sharedPersistentContainerLock { + return kPersistentContainerLock; +} + - (void)awakeFromNib { [super awakeFromNib]; @@ -240,7 +250,9 @@ static void *playlistControllerContext = &playlistControllerContext; - (void)commitPersistentStore { NSError *error = nil; + [self.persistentContainerLock lock]; [self.persistentContainer.viewContext save:&error]; + [self.persistentContainerLock unlock]; if(error) { ALog(@"Error committing playlist storage: %@", [error localizedDescription]); } @@ -256,7 +268,9 @@ static void *playlistControllerContext = &playlistControllerContext; pc.count += 1; pc.lastPlayed = [NSDate date]; } else { + [self.persistentContainerLock lock]; pc = [NSEntityDescription insertNewObjectForEntityForName:@"PlayCount" inManagedObjectContext:self.persistentContainer.viewContext]; + [self.persistentContainerLock unlock]; pc.count = 1; pc.firstSeen = pc.lastPlayed = [NSDate date]; pc.album = pe.album; @@ -272,7 +286,9 @@ static void *playlistControllerContext = &playlistControllerContext; PlayCount *pc = pe.playCountItem; if(!pc) { + [self.persistentContainerLock lock]; pc = [NSEntityDescription insertNewObjectForEntityForName:@"PlayCount" inManagedObjectContext:self.persistentContainer.viewContext]; + [self.persistentContainerLock unlock]; pc.count = 0; pc.firstSeen = [NSDate date]; pc.album = pe.album; @@ -287,7 +303,9 @@ static void *playlistControllerContext = &playlistControllerContext; PlayCount *pc = pe.playCountItem; if(!pc) { + [self.persistentContainerLock lock]; pc = [NSEntityDescription insertNewObjectForEntityForName:@"PlayCount" inManagedObjectContext:self.persistentContainer.viewContext]; + [self.persistentContainerLock unlock]; pc.count = 0; pc.firstSeen = [NSDate date]; pc.album = pe.album; @@ -1341,7 +1359,9 @@ static void *playlistControllerContext = &playlistControllerContext; request.sortDescriptors = @[sortDescriptor]; NSError *error = nil; + [self.persistentContainerLock lock]; NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error]; + [self.persistentContainerLock unlock]; if(results && [results count] > 0) { [queueList removeAllObjects]; @@ -1360,7 +1380,9 @@ static void *playlistControllerContext = &playlistControllerContext; request.sortDescriptors = @[sortDescriptor]; NSError *error = nil; + [self.persistentContainerLock lock]; NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error]; + [self.persistentContainerLock unlock]; if(results && [results count] > 0) { [shuffleList removeAllObjects]; @@ -1887,7 +1909,9 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"SandboxToken"]; NSError *error = nil; + [self.persistentContainerLock lock]; NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error]; + [self.persistentContainerLock unlock]; if(!results || [results count] < 1) return YES; else return NO; diff --git a/Playlist/PlaylistEntry.m b/Playlist/PlaylistEntry.m index 5dd3ae0b0..492524f98 100644 --- a/Playlist/PlaylistEntry.m +++ b/Playlist/PlaylistEntry.m @@ -16,6 +16,7 @@ #import "SHA256Digest.h" #import "SecondsFormatter.h" +extern NSLock *kPersistentContainerLock; extern NSPersistentContainer *kPersistentContainer; extern NSMutableDictionary *kArtworkDictionary; @@ -363,7 +364,9 @@ extern NSMutableDictionary *kArtworkDictionary; self.artHash = imageCacheTag; if(![kArtworkDictionary objectForKey:imageCacheTag]) { + [kPersistentContainerLock lock]; AlbumArtwork *art = [NSEntityDescription insertNewObjectForEntityForName:@"AlbumArtwork" inManagedObjectContext:kPersistentContainer.viewContext]; + [kPersistentContainerLock unlock]; art.artHash = imageCacheTag; art.artData = albumArtInternal; @@ -586,7 +589,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path) { request.predicate = predicate; NSError *error = nil; + [kPersistentContainerLock lock]; NSArray *results = [kPersistentContainer.viewContext executeFetchRequest:request error:&error]; + [kPersistentContainerLock unlock]; if(!results || [results count] < 1) { NSPredicate *filenamePredicate = [NSPredicate predicateWithFormat:@"filename == %@", self.filenameFragment]; @@ -594,7 +599,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path) { request = [NSFetchRequest fetchRequestWithEntityName:@"PlayCount"]; request.predicate = filenamePredicate; + [kPersistentContainerLock lock]; results = [kPersistentContainer.viewContext executeFetchRequest:request error:&error]; + [kPersistentContainerLock unlock]; } if(!results || [results count] < 1) return nil; diff --git a/Playlist/PlaylistLoader.m b/Playlist/PlaylistLoader.m index b6b480970..75d1b97da 100644 --- a/Playlist/PlaylistLoader.m +++ b/Playlist/PlaylistLoader.m @@ -518,7 +518,9 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc __block PlaylistEntry *pe; dispatch_sync_reentrant(dispatch_get_main_queue(), ^{ + [self->playlistController.persistentContainerLock lock]; pe = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:self->playlistController.persistentContainer.viewContext]; + [self->playlistController.persistentContainerLock unlock]; pe.url = url; pe.index = index + i; pe.rawTitle = [[url path] lastPathComponent]; @@ -540,7 +542,9 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc __block PlaylistEntry *pe; dispatch_sync_reentrant(dispatch_get_main_queue(), ^{ + [self->playlistController.persistentContainerLock lock]; pe = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:self->playlistController.persistentContainer.viewContext]; + [self->playlistController.persistentContainerLock unlock]; [pe setValuesForKeysWithDictionary:entry]; pe.index = index + i; pe.queuePosition = -1; @@ -742,7 +746,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path); request.predicate = predicate; NSError *error; + [playlistController.persistentContainerLock lock]; NSArray *results = [moc executeFetchRequest:request error:&error]; + [playlistController.persistentContainerLock unlock]; if(results && [results count] > 0) { for(PlaylistEntry *pe in results) { @@ -869,7 +875,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path); NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"AlbumArtwork"]; NSError *error = nil; + [playlistController.persistentContainerLock lock]; NSArray *results = [moc executeFetchRequest:request error:&error]; + [playlistController.persistentContainerLock unlock]; if(!results) { ALog(@"Error fetching AlbumArtwork objects: %@\n%@", [error localizedDescription], [error userInfo]); abort(); @@ -885,7 +893,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path); request.sortDescriptors = @[sortDescriptor]; + [playlistController.persistentContainerLock lock]; results = [moc executeFetchRequest:request error:&error]; + [playlistController.persistentContainerLock unlock]; if(!results) { ALog(@"Error fetching PlaylistEntry objects: %@\n%@", [error localizedDescription], [error userInfo]); abort(); @@ -899,7 +909,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path); for(PlaylistEntry *pe in resultsCopy) { if(pe.deLeted || !pe.urlString || [pe.urlString length] < 1) { [pruneSet addIndex:index]; + [playlistController.persistentContainerLock lock]; [moc deleteObject:pe]; + [playlistController.persistentContainerLock unlock]; } ++index; } diff --git a/Preferences/Preferences/PathSuggester.m b/Preferences/Preferences/PathSuggester.m index 08e556c87..2673990a3 100644 --- a/Preferences/Preferences/PathSuggester.m +++ b/Preferences/Preferences/PathSuggester.m @@ -55,6 +55,7 @@ [pathsList removeObjects:[pathsList arrangedObjects]]; + NSLock *lock = [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; NSPredicate *hasUrlPredicate = [NSPredicate predicateWithFormat:@"urlString != nil && urlString != %@", @""]; @@ -66,7 +67,9 @@ request.predicate = predicate; NSError *error = nil; + [lock lock]; NSArray *results = [pc.viewContext executeFetchRequest:request error:&error]; + [lock unlock]; if(!results || [results count] < 1) return; diff --git a/Preferences/Preferences/SandboxPathBehaviorController.m b/Preferences/Preferences/SandboxPathBehaviorController.m index 7b02200e6..304ec347a 100644 --- a/Preferences/Preferences/SandboxPathBehaviorController.m +++ b/Preferences/Preferences/SandboxPathBehaviorController.m @@ -32,12 +32,15 @@ NSSortDescriptor *sortDescriptor = [NSSortDescriptor sortDescriptorWithKey:@"path" ascending:YES]; [self setSortDescriptors:@[sortDescriptor]]; + NSLock *lock = [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"SandboxToken"]; NSError *error = nil; + [lock lock]; NSArray *results = [pc.viewContext executeFetchRequest:request error:&error]; + [lock unlock]; if(results && [results count] > 0) { for(SandboxToken *token in results) { @@ -57,14 +60,19 @@ return; } + NSLock *lock = [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; + [lock lock]; SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext]; + [lock unlock]; if(token) { token.path = [url path]; token.bookmark = bookmark; + [lock lock]; [pc.viewContext save:&err]; + [lock unlock]; if(err) { ALog(@"Error saving bookmark: %@", [err localizedDescription]); } else { @@ -74,6 +82,7 @@ } - (void)removeToken:(id)token { + NSLock *lock = [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; NSArray *objects = [[self arrangedObjects] copy]; @@ -83,7 +92,9 @@ for(NSDictionary *obj in objects) { if([[obj objectForKey:@"token"] isEqualTo:token]) { [self removeObject:obj]; + [lock lock]; [pc.viewContext deleteObject:token]; + [lock unlock]; updated = YES; break; } @@ -91,7 +102,9 @@ if(updated) { NSError *error; + [lock lock]; [pc.viewContext save:&error]; + [lock unlock]; if(error) { ALog(@"Error deleting bookmark: %@", [error localizedDescription]); } @@ -100,17 +113,22 @@ - (void)removeStaleEntries { BOOL updated = NO; + NSLock *lock = [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; for(NSDictionary *entry in [[self arrangedObjects] copy]) { if([[entry objectForKey:@"valid"] isEqualToString:NSLocalizedPrefString(@"ValidNo")]) { [self removeObject:entry]; + [lock lock]; [pc.viewContext deleteObject:[entry objectForKey:@"token"]]; + [lock unlock]; updated = YES; } } if(updated) { NSError *error; + [lock lock]; [pc.viewContext save:&error]; + [lock unlock]; if(error) { ALog(@"Error saving after removing stale bookmarks: %@", [error localizedDescription]); } diff --git a/Spotlight/SpotlightPlaylistEntry.m b/Spotlight/SpotlightPlaylistEntry.m index b89cf7049..c6cd2a703 100644 --- a/Spotlight/SpotlightPlaylistEntry.m +++ b/Spotlight/SpotlightPlaylistEntry.m @@ -13,6 +13,7 @@ // with format (entryKey, transformerName) static NSDictionary *importKeys; +extern NSLock *kPersistentContainerLock; extern NSPersistentContainer *kPersistentContainer; @implementation SpotlightPlaylistEntry @@ -37,7 +38,9 @@ extern NSPersistentContainer *kPersistentContainer; } + (PlaylistEntry *)playlistEntryWithMetadataItem:(NSMetadataItem *)metadataItem { + [kPersistentContainerLock lock]; PlaylistEntry *entry = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:kPersistentContainer.viewContext]; + [kPersistentContainerLock unlock]; entry.deLeted = YES; diff --git a/Utils/SQLiteStore.m b/Utils/SQLiteStore.m index 764263ebf..c92bb394d 100644 --- a/Utils/SQLiteStore.m +++ b/Utils/SQLiteStore.m @@ -14,6 +14,7 @@ #import "SHA256Digest.h" +extern NSLock *kPersistentContainerLock; extern NSPersistentContainer *kPersistentContainer; NSString *getDatabasePath(void) { @@ -1443,7 +1444,9 @@ static SQLiteStore *g_sharedStore = nil; #endif - (PlaylistEntry *_Nonnull)getTrack:(int64_t)trackId { + [kPersistentContainerLock lock]; PlaylistEntry *entry = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:kPersistentContainer.viewContext]; + [kPersistentContainerLock unlock]; if(trackId < 0) { entry.error = YES; @@ -1863,7 +1866,9 @@ static SQLiteStore *g_sharedStore = nil; entry.index = index; entry.entryId = entryId; } else { + [kPersistentContainerLock lock]; [kPersistentContainer.viewContext deleteObject:entry]; + [kPersistentContainerLock unlock]; entry = nil; } } diff --git a/Utils/SandboxBroker.m b/Utils/SandboxBroker.m index 036b85dc8..f9dc52587 100644 --- a/Utils/SandboxBroker.m +++ b/Utils/SandboxBroker.m @@ -92,6 +92,10 @@ static SandboxBroker *kSharedSandboxBroker = nil; return kSharedSandboxBroker; } ++ (NSLock *)sharedPersistentContainerLock { + return [NSClassFromString(@"PlaylistController") sharedPersistentContainerLock]; +} + + (NSPersistentContainer *)sharedPersistentContainer { return [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; } @@ -149,6 +153,7 @@ static SandboxBroker *kSharedSandboxBroker = nil; - (SandboxEntry *)recursivePathTest:(NSURL *)url { SandboxEntry *ret = nil; + NSLock *lock = [SandboxBroker sharedPersistentContainerLock]; NSPersistentContainer *pc = [SandboxBroker sharedPersistentContainer]; NSPredicate *folderPredicate = [NSPredicate predicateWithFormat:@"folder == NO"]; @@ -159,7 +164,9 @@ static SandboxBroker *kSharedSandboxBroker = nil; request.predicate = predicate; NSError *error = nil; + [lock lock]; NSArray *results = [pc.viewContext executeFetchRequest:request error:&error]; + [lock unlock]; if(results && [results count] > 0) { ret = [[SandboxEntry alloc] initWithToken:results[0]]; } @@ -174,8 +181,9 @@ static SandboxBroker *kSharedSandboxBroker = nil; request.predicate = predicate; error = nil; + [lock lock]; results = [pc.viewContext executeFetchRequest:request error:&error]; - if(results) results = [results copy]; + [lock unlock]; if(results && [results count] > 0) { for(SandboxToken *token in results) { @@ -240,9 +248,12 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc } dispatch_sync_reentrant(dispatch_get_main_queue(), ^{ - NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; + NSLock *lock = [SandboxBroker sharedPersistentContainerLock]; + NSPersistentContainer *pc = [SandboxBroker sharedPersistentContainer]; + [lock lock]; SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext]; + [lock unlock]; if(token) { token.path = [folderUrl path]; @@ -283,10 +294,13 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc return; } - NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; + NSLock *lock = [SandboxBroker sharedPersistentContainerLock]; + NSPersistentContainer *pc = [SandboxBroker sharedPersistentContainer]; dispatch_sync_reentrant(dispatch_get_main_queue(), ^{ + [lock lock]; SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext]; + [lock unlock]; if(token) { token.path = [url path]; @@ -335,9 +349,12 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc return; } - NSPersistentContainer *pc = [NSClassFromString(@"PlaylistController") sharedPersistentContainer]; + NSLock *lock = [SandboxBroker sharedPersistentContainerLock]; + NSPersistentContainer *pc = [SandboxBroker sharedPersistentContainer]; + [lock lock]; SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext]; + [lock unlock]; if(token) { token.path = [folderUrl path];