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

Caused by a use-after-free of an unretained ivar
sequelpro · Oct 5, 2015 · d4641ec90fe1b50fca0256e26338d955290fd8b7 · d4641ec
1 parent 80f8cae
commit d4641ec90fe1b50fca0256e26338d955290fd8b7
Unified Split

Showing with 48 additions and 6 deletions.

+1 −0 Source/SPDatabaseDocument.m

+5 −1 Source/SPFunctions.h

+10 −0 Source/SPWindowController.h

+30 −4 Source/SPWindowController.m

+2 −1 Source/SPWindowControllerDelegate.m
diff --git a/Source/SPDatabaseDocument.m b/Source/SPDatabaseDocument.m
@@ -6290,6 +6290,7 @@ - (void)_processDatabaseChangedBundleTriggerActions

 - (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
@@ -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
@@ -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
@@ -53,7 +53,8 @@ - (void)_setUpTabBar;
 - (void)_updateProgressIndicatorForItem:(NSTabViewItem *)theItem;
 - (void)_createTitleBarLineHidingView;
 - (void)_updateLineHidingViewState;
-
+- (void)_switchOutSelectedTableDocument:(SPDatabaseDocument *)newDoc;
+- (void)_selectedTableDocumentDeallocd:(NSNotification *)notification;
 @end

 @implementation SPWindowController
@@ -63,7 +64,7 @@ @implementation SPWindowController

 - (void)awakeFromNib
 {
-	selectedTableDocument = nil;
+	[self _switchOutSelectedTableDocument:nil];
 	
 	[[self window] setCollectionBehavior:[[self window] collectionBehavior] | NSWindowCollectionBehaviorFullScreenPrimary];

@@ -141,7 +142,7 @@ - (SPDatabaseDocument *)selectedTableDocument
 */
 - (void)updateSelectedTableDocument
 {
-	selectedTableDocument = [[tabView selectedTabViewItem] identifier];
+	[self _switchOutSelectedTableDocument:[[tabView selectedTabViewItem] identifier]];
 	
 	[selectedTableDocument didBecomeActiveTabInWindow];
 }
@@ -388,7 +389,7 @@ - (NSMethodSignature *)methodSignatureForSelector:(SEL)theSelector
 */
 - (BOOL)respondsToSelector:(SEL)theSelector
 {
-	return ([super respondsToSelector:theSelector] || (selectedTableDocument && [selectedTableDocument respondsToSelector:theSelector]));
+	return ([super respondsToSelector:theSelector] || [selectedTableDocument respondsToSelector:theSelector]);
 }

 /**
@@ -529,10 +530,35 @@ - (void)_updateLineHidingViewState
 	}
 }

+
+- (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
@@ -41,6 +41,7 @@ @interface SPWindowController (SPDeclaredAPI)

 - (void)_updateProgressIndicatorForItem:(NSTabViewItem *)theItem;
 - (void)_updateLineHidingViewState;
+- (void)_switchOutSelectedTableDocument:(SPDatabaseDocument *)newDoc;

 @end

@@ -186,7 +187,7 @@ - (void)tabView:(NSTabView *)tabView didSelectTabViewItem:(NSTabViewItem *)tabVi
 {
 	if ([[PSMTabDragAssistant sharedDragAssistant] isDragging]) return;
 	
-	selectedTableDocument = [tabViewItem identifier];
+	[self _switchOutSelectedTableDocument:[tabViewItem identifier]];
 	[selectedTableDocument didBecomeActiveTabInWindow];

 	if ([[self window] isKeyWindow]) [selectedTableDocument tabDidBecomeKey];