Better locking behavior for playlist storage

This should fix up potential locking issues with maintaining a copy of
the results set while certain other background actions may happen, such
as the player updating play counts while playing.

Signed-off-by: Christopher Snowhill <kode54@gmail.com>

Application/AppController.m

@@ -225,6 +225,9 @@ static AppController *kAppController = nil;
 		NSError *error = nil;
 		[playlistController.persistentContainerLock lock];
 		NSArray *results = [playlistController.persistentContainer.viewContext executeFetchRequest:request error:&error];
+		if(results) {
+			results = [results copy];
+		}
 		[playlistController.persistentContainerLock unlock];
 
 		if(results && [results count] == 1) {

Playlist/PlaylistController.m

@@ -267,18 +267,20 @@ static void *playlistControllerContext = &playlistControllerContext;
 	PlayCount *pc = pe.playCountItem;
 
 	if(pc) {
+		[self.persistentContainerLock lock];
 		pc.count += 1;
 		pc.lastPlayed = [NSDate date];
+		[self.persistentContainerLock unlock];
 	} 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;
 		pc.artist = pe.artist;
 		pc.title = pe.title;
 		pc.filename = pe.filenameFragment;
+		[self.persistentContainerLock unlock];
 	}
 
 	[self commitPersistentStore];
@@ -290,13 +292,13 @@ static void *playlistControllerContext = &playlistControllerContext;
 	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;
 		pc.artist = pe.artist;
 		pc.title = pe.title;
 		pc.filename = pe.filenameFragment;
+		[self.persistentContainerLock unlock];
 	}
 }
 
@@ -307,16 +309,18 @@ static void *playlistControllerContext = &playlistControllerContext;
 		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;
 			pc.artist = pe.artist;
 			pc.title = pe.title;
 			pc.filename = pe.filenameFragment;
+			[self.persistentContainerLock unlock];
 		}
 
+		[self.persistentContainerLock lock];
 		pc.rating = rating;
+		[self.persistentContainerLock unlock];
 
 		[self commitPersistentStore];
 	}
@@ -1380,6 +1384,9 @@ static void *playlistControllerContext = &playlistControllerContext;
 	NSError *error = nil;
 	[self.persistentContainerLock lock];
 	NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error];
+    if(results) {
+		results = [results copy];
+    }
 	[self.persistentContainerLock unlock];
 
 	if(results && [results count] > 0) {
@@ -1401,6 +1408,9 @@ static void *playlistControllerContext = &playlistControllerContext;
 	NSError *error = nil;
 	[self.persistentContainerLock lock];
 	NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error];
+    if(results) {
+		results = [results copy];
+    }
 	[self.persistentContainerLock unlock];
 
 	if(results && [results count] > 0) {
@@ -1937,15 +1947,18 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 }
 
 - (BOOL)pathSuggesterEmpty {
+	BOOL rval;
+
 	NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"SandboxToken"];
 
 	NSError *error = nil;
 	[self.persistentContainerLock lock];
 	NSArray *results = [self.persistentContainer.viewContext executeFetchRequest:request error:&error];
+	if(!results || [results count] < 1) rval = YES;
+	else rval = NO;
 	[self.persistentContainerLock unlock];
 
-	if(!results || [results count] < 1) return YES;
+	return rval;
-	else return NO;
 }
 
 @end

Playlist/PlaylistEntry.m

@@ -366,9 +366,9 @@ extern NSMutableDictionary<NSString *, AlbumArtwork *> *kArtworkDictionary;
 	if(![kArtworkDictionary objectForKey:imageCacheTag]) {
 		[kPersistentContainerLock lock];
 		AlbumArtwork *art = [NSEntityDescription insertNewObjectForEntityForName:@"AlbumArtwork" inManagedObjectContext:kPersistentContainer.viewContext];
-		[kPersistentContainerLock unlock];
 		art.artHash = imageCacheTag;
 		art.artData = albumArtInternal;
+		[kPersistentContainerLock unlock];
 
 		[kArtworkDictionary setObject:art forKey:imageCacheTag];
 	}
@@ -591,7 +591,6 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path) {
 	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];
@@ -599,10 +598,13 @@ 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 = [results copy];
+	}
+	[kPersistentContainerLock unlock];
 
 	if(!results || [results count] < 1) return nil;
 

Playlist/PlaylistLoader.m

@@ -569,11 +569,11 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 		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];
 			pe.queuePosition = -1;
+			[self->playlistController.persistentContainerLock unlock];
 		});
 
 		[entries addObject:pe];
@@ -593,10 +593,10 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 			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;
+				[self->playlistController.persistentContainerLock unlock];
 			});
 
 			[entries addObject:pe];
@@ -797,6 +797,9 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path);
 	NSError *error;
 	[playlistController.persistentContainerLock lock];
 	NSArray *results = [moc executeFetchRequest:request error:&error];
+	if(results) {
+		results = [results copy];
+	}
 	[playlistController.persistentContainerLock unlock];
 
 	if(results && [results count] > 0) {
@@ -926,8 +929,8 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path);
 		NSError *error = nil;
 		[playlistController.persistentContainerLock lock];
 		NSArray *results = [moc executeFetchRequest:request error:&error];
-		[playlistController.persistentContainerLock unlock];
 		if(!results) {
+			[playlistController.persistentContainerLock unlock];
 			ALog(@"Error fetching AlbumArtwork objects: %@\n%@", [error localizedDescription], [error userInfo]);
 			abort();
 		}
@@ -935,6 +938,7 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path);
 		for(AlbumArtwork *art in results) {
 			[kArtworkDictionary setObject:art forKey:art.artHash];
 		}
+		[playlistController.persistentContainerLock unlock];
 
 		request = [NSFetchRequest fetchRequestWithEntityName:@"PlaylistEntry"];
 
@@ -944,15 +948,20 @@ NSURL *_Nullable urlForPath(NSString *_Nullable path);
 
 		[playlistController.persistentContainerLock lock];
 		results = [moc executeFetchRequest:request error:&error];
-		[playlistController.persistentContainerLock unlock];
 		if(!results) {
+			[playlistController.persistentContainerLock unlock];
 			ALog(@"Error fetching PlaylistEntry objects: %@\n%@", [error localizedDescription], [error userInfo]);
 			abort();
 		}
 
-		if([results count] == 0) return NO;
+		if([results count] == 0) {
+			[playlistController.persistentContainerLock unlock];
+			return NO;
+		}
 
 		NSMutableArray *resultsCopy = [results mutableCopy];
+		[playlistController.persistentContainerLock unlock];
+
 		NSMutableIndexSet *pruneSet = [[NSMutableIndexSet alloc] init];
 		NSUInteger index = 0;
 		for(PlaylistEntry *pe in resultsCopy) {

Preferences/Preferences/PathSuggester.m

@@ -69,6 +69,9 @@
 	NSError *error = nil;
 	[lock lock];
 	NSArray *results = [pc.viewContext executeFetchRequest:request error:&error];
+	if(results) {
+		results = [results copy];
+	}
 	[lock unlock];
 
 	if(!results || [results count] < 1) return;

Preferences/Preferences/SandboxPathBehaviorController.m

@@ -44,6 +44,9 @@
 	NSError *error = nil;
 	[lock lock];
 	NSArray *results = [pc.viewContext executeFetchRequest:request error:&error];
+	if(results) {
+		results = [results copy];
+	}
 	[lock unlock];
 
 	if(results && [results count] > 0) {
@@ -69,12 +72,10 @@
 
 	[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) {
@@ -84,6 +85,8 @@
 			[NSClassFromString(@"SandboxBroker") cleanupFolderAccess];
 			[self refresh];
 		}
+	} else {
+		[lock unlock];
 	}
 }
 

Spotlight/SpotlightPlaylistEntry.m

@@ -40,7 +40,6 @@ extern NSPersistentContainer *kPersistentContainer;
 + (PlaylistEntry *)playlistEntryWithMetadataItem:(NSMetadataItem *)metadataItem {
 	[kPersistentContainerLock lock];
 	PlaylistEntry *entry = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:kPersistentContainer.viewContext];
-	[kPersistentContainerLock unlock];
 
 	entry.deLeted = YES;
 
@@ -85,6 +84,7 @@ extern NSPersistentContainer *kPersistentContainer;
 	entry.url = url;
 
 	[entry setMetadata:dict];
+	[kPersistentContainerLock unlock];
 
 	return entry;
 }

Utils/SQLiteStore.m

@@ -1446,12 +1446,12 @@ static SQLiteStore *g_sharedStore = nil;
 - (PlaylistEntry *_Nonnull)getTrack:(int64_t)trackId {
 	[kPersistentContainerLock lock];
 	PlaylistEntry *entry = [NSEntityDescription insertNewObjectForEntityForName:@"PlaylistEntry" inManagedObjectContext:kPersistentContainer.viewContext];
-	[kPersistentContainerLock unlock];
 
 	if(trackId < 0) {
 		entry.error = YES;
 		entry.errorMessage = NSLocalizedString(@"ErrorInvalidTrackId", @"");
 		entry.deLeted = YES;
+		[kPersistentContainerLock unlock];
 		return entry;
 	}
 
@@ -1462,6 +1462,7 @@ static SQLiteStore *g_sharedStore = nil;
 		entry.error = YES;
 		entry.errorMessage = NSLocalizedString(@"ErrorSqliteProblem", @"");
 		entry.deLeted = YES;
+		[kPersistentContainerLock unlock];
 		return entry;
 	}
 
@@ -1472,6 +1473,7 @@ static SQLiteStore *g_sharedStore = nil;
 		entry.error = YES;
 		entry.errorMessage = NSLocalizedString(@"ErrorSqliteProblem", @"");
 		entry.deLeted = YES;
+		[kPersistentContainerLock unlock];
 		return entry;
 	}
 
@@ -1547,6 +1549,7 @@ static SQLiteStore *g_sharedStore = nil;
 		entry.errorMessage = NSLocalizedString(@"ErrorTrackMissing", @"");
 		entry.deLeted = YES;
 	}
+	[kPersistentContainerLock unlock];
 
 	sqlite3_reset(st);
 
@@ -1863,8 +1866,10 @@ static SQLiteStore *g_sharedStore = nil;
 		int64_t entryId = sqlite3_column_int64(st, select_playlist_out_entry_id);
 		entry = [self getTrack:trackId];
 		if(!entry.deLeted && !entry.error) {
+			[kPersistentContainerLock lock];
 			entry.index = index;
 			entry.entryId = entryId;
+			[kPersistentContainerLock unlock];
 		} else {
 			[kPersistentContainerLock lock];
 			[kPersistentContainer.viewContext deleteObject:entry];

Utils/SandboxBroker.m

@@ -166,6 +166,9 @@ static SandboxBroker *kSharedSandboxBroker = nil;
 	NSError *error = nil;
 	[lock lock];
 	NSArray *results = [pc.viewContext executeFetchRequest:request error:&error];
+	if(results) {
+		results = [results copy];
+	}
 	[lock unlock];
 	if(results && [results count] > 0) {
 		ret = [[SandboxEntry alloc] initWithToken:results[0]];
@@ -183,6 +186,9 @@ static SandboxBroker *kSharedSandboxBroker = nil;
 		error = nil;
 		[lock lock];
 		results = [pc.viewContext executeFetchRequest:request error:&error];
+		if(results) {
+			results = [results copy];
+		}
 		[lock unlock];
 
 		if(results && [results count] > 0) {
@@ -253,12 +259,14 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 
 				[lock lock];
 				SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext];
-				[lock unlock];
 
 				if(token) {
 					token.path = [folderUrl path];
 					token.bookmark = bookmark;
+					[lock unlock];
 					[SandboxBroker cleanupFolderAccess];
+				} else {
+					[lock unlock];
 				}
 			});
 		}
@@ -301,13 +309,13 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 			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];
 					token.bookmark = bookmark;
 					token.folder = NO;
 				}
+				[lock unlock];
 			});
 		}
 	}
@@ -369,12 +377,14 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 
 					[lock lock];
 					SandboxToken *token = [NSEntityDescription insertNewObjectForEntityForName:@"SandboxToken" inManagedObjectContext:pc.viewContext];
-					[lock unlock];
 
 					if(token) {
 						token.path = [folderUrl path];
 						token.bookmark = bookmark;
+						[lock unlock];
 						[SandboxBroker cleanupFolderAccess];
+					} else {
+						[lock unlock];
 					}
 				}
 			});
@@ -391,14 +401,17 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc
 	request.sortDescriptors = @[sortDescriptor];
 
 	NSError *error = nil;
+	NSMutableArray *resultsCopy = nil;
 	[lock lock];
 	NSArray *results = [pc.viewContext executeFetchRequest:request error:&error];
+	if(results) {
+		resultsCopy = [results mutableCopy];
+	}
 	[lock unlock];
 
 	BOOL isUpdated = NO;
 
-	if(results && [results count]) {
+	if(resultsCopy && [resultsCopy count]) {
-		NSMutableArray *resultsCopy = [results mutableCopy];
 		for(NSUInteger i = 0; i < [resultsCopy count] - 1; ++i) {
 			SandboxToken *token = resultsCopy[i];
 			NSURL *url = [NSURL fileURLWithPath:token.path];