Trying to fix a range of crashes when closing a connection window

Caused by a use-after-free of an unretained ivar

5 files changed, 48 insertions, 6 deletions

diff --git a/Source/SPDatabaseDocument.m b/Source/SPDatabaseDocument.m
index 5c0553f8..dd3baae3 100644
--- a/Source/SPDatabaseDocument.m
+++ b/Source/SPDatabaseDocument.m
@@ -6290,6 +6290,7 @@ static int64_t SPDatabaseDocumentInstanceCounter = 0;
 
 - (void)dealloc
 {
+	NSAssert([NSThread isMainThread], @"Calling %s from a background thread is not supported!",__func__);
 #ifndef SP_CODA /* Unregister observers */
 	// Unregister observers
 	[prefs removeObserver:self forKeyPath:SPDisplayTableViewVerticalGridlines];
diff --git a/Source/SPFunctions.h b/Source/SPFunctions.h
index f2ca9c7e..b68964ca 100644
--- a/Source/SPFunctions.h
+++ b/Source/SPFunctions.h
@@ -28,6 +28,10 @@
 //
 //  More info at <https://github.com/sequelpro/sequelpro>
 
-
+/**
+ * Synchronously execute a block on the main thread.
+ * This function can be called from a background thread as well as from
+ * the main thread.
+ */
 void SPMainQSync(void (^block)(void));
 
diff --git a/Source/SPWindowController.h b/Source/SPWindowController.h
index c99a2ff2..a86a85f9 100644
--- a/Source/SPWindowController.h
+++ b/Source/SPWindowController.h
@@ -49,7 +49,17 @@
 // Database connection management
 - (IBAction)addNewConnection:(id)sender;
 - (IBAction)moveSelectedTabInNewWindow:(id)sender;
+
+/**
+ * @danger THIS IS NOT RETAINED!!! 
+ *
+ * Ever only directly use it on the main thread! 
+ * Do not cache it without retaining first!
+ * For background threads get it and retain it via the main thread!
+ *   Release it on the main thread again.
+ */
 - (SPDatabaseDocument *)selectedTableDocument;
+
 - (void)updateSelectedTableDocument;
 - (void)updateAllTabTitles:(id)sender;
 - (IBAction)closeTab:(id)sender;
diff --git a/Source/SPWindowController.m b/Source/SPWindowController.m
index db94f5ce..68c89ad8 100644
--- a/Source/SPWindowController.m
+++ b/Source/SPWindowController.m
@@ -53,7 +53,8 @@ enum {
 - (void)_updateProgressIndicatorForItem:(NSTabViewItem *)theItem;
 - (void)_createTitleBarLineHidingView;
 - (void)_updateLineHidingViewState;
-
+- (void)_switchOutSelectedTableDocument:(SPDatabaseDocument *)newDoc;
+- (void)_selectedTableDocumentDeallocd:(NSNotification *)notification;
 @end
 
 @implementation SPWindowController
@@ -63,7 +64,7 @@ enum {
 
 - (void)awakeFromNib
 {
-	selectedTableDocument = nil;
+	[self _switchOutSelectedTableDocument:nil];
 	
 	[[self window] setCollectionBehavior:[[self window] collectionBehavior] | NSWindowCollectionBehaviorFullScreenPrimary];
 
@@ -141,7 +142,7 @@ enum {
  */
 - (void)updateSelectedTableDocument
 {
-	selectedTableDocument = [[tabView selectedTabViewItem] identifier];
+	[self _switchOutSelectedTableDocument:[[tabView selectedTabViewItem] identifier]];
 	
 	[selectedTableDocument didBecomeActiveTabInWindow];
 }
@@ -388,7 +389,7 @@ enum {
  */
 - (BOOL)respondsToSelector:(SEL)theSelector
 {
-	return ([super respondsToSelector:theSelector] || (selectedTableDocument && [selectedTableDocument respondsToSelector:theSelector]));
+	return ([super respondsToSelector:theSelector] || [selectedTableDocument respondsToSelector:theSelector]);
 }
 
 /**
@@ -529,10 +530,35 @@ enum {
 	}
 }
 
+
+- (void)_switchOutSelectedTableDocument:(SPDatabaseDocument *)newDoc
+{
+	NSAssert([NSThread isMainThread], @"Switching the selectedTableDocument via a background thread is not supported!");
+	
+	// shortcut if there is nothing to do
+	if(selectedTableDocument == newDoc) return;
+	
+	NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];
+	if(selectedTableDocument) {
+		[nc removeObserver:self name:SPDocumentWillCloseNotification object:selectedTableDocument];
+		selectedTableDocument = nil;
+	}
+	if(newDoc) {
+		[nc addObserver:self selector:@selector(_selectedTableDocumentDeallocd:) name:SPDocumentWillCloseNotification object:newDoc];
+		selectedTableDocument = newDoc;
+	}
+}
+
+- (void)_selectedTableDocumentDeallocd:(NSNotification *)notification
+{
+	[self _switchOutSelectedTableDocument:nil];
+}
+
 #pragma mark -
 
 - (void)dealloc
 {
+	[self _switchOutSelectedTableDocument:nil];
 	[[NSNotificationCenter defaultCenter] removeObserver:self];
 	
 	[NSObject cancelPreviousPerformRequestsWithTarget:self];
diff --git a/Source/SPWindowControllerDelegate.m b/Source/SPWindowControllerDelegate.m
index 47959f1b..3e94cebf 100644
--- a/Source/SPWindowControllerDelegate.m
+++ b/Source/SPWindowControllerDelegate.m
@@ -41,6 +41,7 @@
 
 - (void)_updateProgressIndicatorForItem:(NSTabViewItem *)theItem;
 - (void)_updateLineHidingViewState;
+- (void)_switchOutSelectedTableDocument:(SPDatabaseDocument *)newDoc;
 
 @end
 
@@ -186,7 +187,7 @@
 {
 	if ([[PSMTabDragAssistant sharedDragAssistant] isDragging]) return;
 	
-	selectedTableDocument = [tabViewItem identifier];
+	[self _switchOutSelectedTableDocument:[tabViewItem identifier]];
 	[selectedTableDocument didBecomeActiveTabInWindow];
 
 	if ([[self window] isKeyWindow]) [selectedTableDocument tabDidBecomeKey];