Further improvements to further address Issue #1332:

 - Ensure that favourites are saved synchronously on exit to avoid background threads being killed
 - Improve logging on favourite rename error
 - Only save favourites on exit if a connection window is open

3 files changed, 46 insertions, 20 deletions

diff --git a/Source/SPAppController.m b/Source/SPAppController.m
index ed8fd400..ca5887c9 100644
--- a/Source/SPAppController.m
+++ b/Source/SPAppController.m
@@ -2236,27 +2236,37 @@ YY_BUFFER_STATE yy_scan_string (const char *);
  */
 - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication *)sender
 {
+	BOOL shouldSaveFavorites = NO;
+
 	if (lastBundleBlobFilesDirectory != nil) {
 		[[NSFileManager defaultManager] removeItemAtPath:lastBundleBlobFilesDirectory error:nil];
 	}
 
-	// Kill all registered BASH commands
+	// Iterate through each open window
 	for (NSWindow *aWindow in [NSApp orderedWindows]) 
 	{
-		if ([[aWindow windowController] isMemberOfClass:[SPWindowController class]]) {
-			for (SPDatabaseDocument *doc in [[aWindow windowController] documents]) 
+		if (![[aWindow windowController] isMemberOfClass:[SPWindowController class]]) continue;
+
+		// Iterate through each document in the window
+		for (SPDatabaseDocument *doc in [[aWindow windowController] documents]) 
+		{
+
+			// Kill any BASH commands which are currently active
+			for (NSDictionary* cmd in [doc runningActivities]) 
 			{
-				for (NSDictionary* cmd in [doc runningActivities]) 
-				{
-					NSInteger pid = [[cmd objectForKey:@"pid"] intValue];
-					NSTask *killTask = [[NSTask alloc] init];
-					
-					[killTask setLaunchPath:@"/bin/sh"];
-					[killTask setArguments:[NSArray arrayWithObjects:@"-c", [NSString stringWithFormat:@"kill -9 -%ld", pid], nil]];
-					[killTask launch];
-					[killTask waitUntilExit];
-					[killTask release];
-				}
+				NSInteger pid = [[cmd objectForKey:@"pid"] intValue];
+				NSTask *killTask = [[NSTask alloc] init];
+				
+				[killTask setLaunchPath:@"/bin/sh"];
+				[killTask setArguments:[NSArray arrayWithObjects:@"-c", [NSString stringWithFormat:@"kill -9 -%ld", pid], nil]];
+				[killTask launch];
+				[killTask waitUntilExit];
+				[killTask release];
+			}
+
+			// If the connection view is active, mark the favourites for saving
+			if (![doc getConnection]) {
+				shouldSaveFavorites = YES;
 			}
 		}
 	}
@@ -2278,8 +2288,10 @@ YY_BUFFER_STATE yy_scan_string (const char *);
 		[c release];
 	}
 	
-	// Make sure we save any changes made to the connection outline view's state
-	[[SPFavoritesController sharedFavoritesController] saveFavorites];
+	// If required, make sure we save any changes made to the connection outline view's state
+	if (shouldSaveFavorites) {
+		[[SPFavoritesController sharedFavoritesController] saveFavoritesSynchronously];
+	}
 
 	return YES;
 
diff --git a/Source/SPFavoritesController.h b/Source/SPFavoritesController.h
index e15ab385..bccc314e 100644
--- a/Source/SPFavoritesController.h
+++ b/Source/SPFavoritesController.h
@@ -57,6 +57,7 @@
 + (SPFavoritesController *)sharedFavoritesController;
 
 - (void)saveFavorites;
+- (void)saveFavoritesSynchronously;
 - (void)reloadFavoritesWithSave:(BOOL)save;
 
 - (SPTreeNode *)addGroupNodeWithName:(NSString *)name asChildOfNode:(SPTreeNode *)parent;
diff --git a/Source/SPFavoritesController.m b/Source/SPFavoritesController.m
index da8be7c6..4e1ccdef 100644
--- a/Source/SPFavoritesController.m
+++ b/Source/SPFavoritesController.m
@@ -35,7 +35,7 @@ static SPFavoritesController *sharedFavoritesController = nil;
 
 - (void)_loadFavorites;
 - (void)_constructFavoritesTree;
-- (void)_saveFavoritesDataInBackground:(NSDictionary *)data;
+- (void)_saveFavoritesData:(NSDictionary *)data;
 - (void)_addNode:(SPTreeNode *)node asChildOfNode:(SPTreeNode *)parent;
 
 - (SPTreeNode *)_constructBranchForNodeData:(NSDictionary *)nodeData;
@@ -101,12 +101,13 @@ static SPFavoritesController *sharedFavoritesController = nil;
  * Saves the current favorites dictionary in memory to disk. Note that the current favorites data file is moved
  * rather than overwritten in the event that we can't write the new file, the original can simply be restored.
  * This method also does a lot of error checking to ensure we don't lose the user's favorites data.
+ * Saves the data in the background so any UI tasks can stay responsive.
  */
 - (void)saveFavorites
 {
 	pthread_mutex_lock(&favoritesLock);
 	
-	[NSThread detachNewThreadSelector:@selector(_saveFavoritesDataInBackground:) 
+	[NSThread detachNewThreadSelector:@selector(_saveFavoritesData:) 
 							 toTarget:self 
 						   withObject:[[[favoritesTree childNodes] objectAtIndex:0] dictionaryRepresentation]];
 	
@@ -114,6 +115,14 @@ static SPFavoritesController *sharedFavoritesController = nil;
 }
 
 /**
+ * Save the current favorites dictionary in memory to disk, in the foreground, in a blocking manner.
+ */
+- (void)saveFavoritesSynchronously
+{
+	[self _saveFavoritesData:[[[favoritesTree childNodes] objectAtIndex:0] dictionaryRepresentation]];
+}
+
+/**
  * Reloads the favorites data from disk with the option to save before doing so.
  *
  * @param save Indicates whether the current favorites data in memory should be saved to disk before being
@@ -343,7 +352,7 @@ static SPFavoritesController *sharedFavoritesController = nil;
  *
  * @param data The raw plist data (serialized NSDictionary) to be saved 
  */
-- (void)_saveFavoritesDataInBackground:(NSDictionary *)data
+- (void)_saveFavoritesData:(NSDictionary *)data
 {
 	NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
 	
@@ -411,7 +420,11 @@ static SPFavoritesController *sharedFavoritesController = nil;
 		NSLog(@"Error writing favorites data. Restoring backup if available: %@", [error localizedDescription]);
 		
 		// Restore the original data file
-		[fileManager moveItemAtPath:favoritesBackupFile toPath:favoritesFile error:NULL];
+		error = nil;
+		[fileManager moveItemAtPath:favoritesBackupFile toPath:favoritesFile error:&error];
+		if (error) {
+			NSLog(@"Could not restore backup; favorites.plist left renamed as %@ due to error (%@)", favoritesBackupFile, [error localizedDescription]);
+		}
 	}
 	else {