From 01767d392e5d42ca199efee3229dd9f8eb36fe06 Mon Sep 17 00:00:00 2001 From: Matt Lilek Date: Tue, 17 Mar 2015 17:31:58 -0700 Subject: [PATCH 1/3] PBViewControllers contain a strong reference to a PBGitWindowController 'super controller', and the only place these objects are ever created is within the PBGitWindowController...which contains a strong, owning reference to these objects creating a retain cycle. --- Classes/Controllers/PBViewController.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Controllers/PBViewController.h b/Classes/Controllers/PBViewController.h index dc574d949..76590f6ec 100644 --- a/Classes/Controllers/PBViewController.h +++ b/Classes/Controllers/PBViewController.h @@ -12,7 +12,7 @@ @interface PBViewController : NSViewController { __weak PBGitRepository *repository; - PBGitWindowController *superController; + __weak PBGitWindowController *superController; NSString *status; BOOL isBusy; From 56949f864aedbfed7a1d4d0a917d4f478e3140a4 Mon Sep 17 00:00:00 2001 From: Matt Lilek Date: Tue, 17 Mar 2015 17:32:06 -0700 Subject: [PATCH 2/3] Stop leaking the repository's PBGitHistoryList and many related objects by breaking a retain cycle between the PBGitHistoryList and its internal PBGitHistoryGrapher. The grapher maintained a strong reference to it's "delegate", but is only ever owned by the same object. Make the "delegate" a weak reference. The two places it's used, create a strong reference while it's used. Finally, -[PBGitRepository close] calls -[PBGitHistoryList cleanup] but so does -[PBGitHistoryList dealloc]. Avoid an uncaught exception/crash by nil-ing out the currentRevList after we've cleaned up as it's no longer valid. --- Classes/git/PBGitHistoryGrapher.h | 2 +- Classes/git/PBGitHistoryGrapher.m | 6 ++++-- Classes/git/PBGitHistoryList.m | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Classes/git/PBGitHistoryGrapher.h b/Classes/git/PBGitHistoryGrapher.h index 0e02e6a3e..06a475531 100644 --- a/Classes/git/PBGitHistoryGrapher.h +++ b/Classes/git/PBGitHistoryGrapher.h @@ -17,7 +17,7 @@ @interface PBGitHistoryGrapher : NSObject { - id delegate; + __weak id delegate; NSOperationQueue *currentQueue; NSMutableSet *searchSHAs; diff --git a/Classes/git/PBGitHistoryGrapher.m b/Classes/git/PBGitHistoryGrapher.m index 7fea53cd7..95b566a56 100644 --- a/Classes/git/PBGitHistoryGrapher.m +++ b/Classes/git/PBGitHistoryGrapher.m @@ -30,7 +30,8 @@ - (id) initWithBaseCommits:(NSSet *)commits viewAllBranches:(BOOL)viewAll queue: - (void)sendCommits:(NSArray *)commits { NSDictionary *commitData = [NSDictionary dictionaryWithObjectsAndKeys:currentQueue, kCurrentQueueKey, commits, kNewCommitsKey, nil]; - [delegate performSelectorOnMainThread:@selector(updateCommitsFromGrapher:) withObject:commitData waitUntilDone:NO]; + id strongDelegate = delegate; + [strongDelegate performSelectorOnMainThread:@selector(updateCommitsFromGrapher:) withObject:commitData waitUntilDone:NO]; } @@ -39,6 +40,7 @@ - (void) graphCommits:(NSArray *)revList if (!revList || [revList count] == 0) return; + id strongDelegate = delegate; //NSDate *start = [NSDate date]; NSThread *currentThread = [NSThread currentThread]; NSDate *lastUpdate = [NSDate date]; @@ -69,7 +71,7 @@ - (void) graphCommits:(NSArray *)revList //NSLog(@"Graphed %i commits in %f seconds (%f/sec)", counter, duration, counter/duration); [self sendCommits:commits]; - [delegate performSelectorOnMainThread:@selector(finishedGraphing) withObject:nil waitUntilDone:NO]; + [strongDelegate performSelectorOnMainThread:@selector(finishedGraphing) withObject:nil waitUntilDone:NO]; } diff --git a/Classes/git/PBGitHistoryList.m b/Classes/git/PBGitHistoryList.m index 72387f301..3b217f2d0 100644 --- a/Classes/git/PBGitHistoryList.m +++ b/Classes/git/PBGitHistoryList.m @@ -92,6 +92,7 @@ - (void)cleanup if (currentRevList) { [currentRevList removeObserver:self forKeyPath:@"commits"]; [currentRevList cancel]; + currentRevList = nil; } [graphQueue cancelAllOperations]; From 9230de7c49bdd78b95ec08cce4c666dba4421441 Mon Sep 17 00:00:00 2001 From: Matt Lilek Date: Tue, 17 Mar 2015 17:32:09 -0700 Subject: [PATCH 3/3] Most IBOutlets should be weak. This breaks many retain cycles and means many more objects are getting released at the proper time. --- Classes/Controllers/PBGitHistoryController.h | 40 +++++++++---------- Classes/Controllers/PBGitSidebarController.h | 10 ++--- Classes/Controllers/PBGitWindowController.h | 15 +++---- Classes/Controllers/PBGitWindowController.m | 6 --- .../Controllers/PBHistorySearchController.h | 12 +++--- Classes/Controllers/PBRefController.h | 8 ++-- Classes/Controllers/PBWebController.h | 6 +-- Classes/Controllers/PBWebHistoryController.h | 4 +- Classes/PBCommitList.h | 8 ++-- Classes/Views/GLFileView.h | 8 ++-- Classes/Views/PBGitRevisionCell.h | 4 +- Classes/Views/PBQLOutlineView.h | 2 +- Classes/Views/PBQLTextView.h | 2 +- 13 files changed, 56 insertions(+), 69 deletions(-) diff --git a/Classes/Controllers/PBGitHistoryController.h b/Classes/Controllers/PBGitHistoryController.h index eeac2b146..ab15f74c1 100644 --- a/Classes/Controllers/PBGitHistoryController.h +++ b/Classes/Controllers/PBGitHistoryController.h @@ -24,33 +24,31 @@ @class PBHistorySearchController; @interface PBGitHistoryController : PBViewController { - IBOutlet PBRefController *refController; - IBOutlet NSSearchField *searchField; - IBOutlet NSArrayController* commitController; - IBOutlet NSTreeController* treeController; - IBOutlet NSOutlineView* fileBrowser; - NSArray *currentFileBrowserSelectionPath; - IBOutlet PBCommitList* commitList; - IBOutlet PBCollapsibleSplitView *historySplitView; + IBOutlet NSArrayController *commitController; + IBOutlet NSTreeController *treeController; IBOutlet PBWebHistoryController *webHistoryController; - QLPreviewPanel* previewPanel; - IBOutlet PBHistorySearchController *searchController; IBOutlet GLFileView *fileView; + IBOutlet PBRefController *refController; + IBOutlet PBHistorySearchController *searchController; - IBOutlet PBGitGradientBarView *upperToolbarView; - IBOutlet NSButton *mergeButton; - IBOutlet NSButton *cherryPickButton; - IBOutlet NSButton *rebaseButton; - - IBOutlet PBGitGradientBarView *scopeBarView; - IBOutlet NSButton *allBranchesFilterItem; - IBOutlet NSButton *localRemoteBranchesFilterItem; - IBOutlet NSButton *selectedBranchFilterItem; + __weak IBOutlet NSSearchField *searchField; + __weak IBOutlet NSOutlineView *fileBrowser; + __weak IBOutlet PBCommitList *commitList; + __weak IBOutlet PBCollapsibleSplitView *historySplitView; + __weak IBOutlet PBGitGradientBarView *upperToolbarView; + __weak IBOutlet NSButton *mergeButton; + __weak IBOutlet NSButton *cherryPickButton; + __weak IBOutlet NSButton *rebaseButton; + __weak IBOutlet PBGitGradientBarView *scopeBarView; + __weak IBOutlet NSButton *allBranchesFilterItem; + __weak IBOutlet NSButton *localRemoteBranchesFilterItem; + __weak IBOutlet NSButton *selectedBranchFilterItem; + __weak IBOutlet id webView; - IBOutlet id webView; + NSArray *currentFileBrowserSelectionPath; + QLPreviewPanel* previewPanel; int selectedCommitDetailsIndex; BOOL forceSelectionUpdate; - PBGitTree *gitTree; PBGitCommit *webCommit; PBGitCommit *selectedCommit; diff --git a/Classes/Controllers/PBGitSidebarController.h b/Classes/Controllers/PBGitSidebarController.h index e28936c33..532ca9dea 100644 --- a/Classes/Controllers/PBGitSidebarController.h +++ b/Classes/Controllers/PBGitSidebarController.h @@ -14,11 +14,11 @@ @class PBGitCommitController; @interface PBGitSidebarController : PBViewController { - IBOutlet NSWindow *window; - IBOutlet NSOutlineView *sourceView; - IBOutlet NSView *sourceListControlsView; - IBOutlet NSPopUpButton *actionButton; - IBOutlet NSSegmentedControl *remoteControls; + __weak IBOutlet NSWindow *window; + __weak IBOutlet NSOutlineView *sourceView; + __weak IBOutlet NSView *sourceListControlsView; + __weak IBOutlet NSPopUpButton *actionButton; + __weak IBOutlet NSSegmentedControl *remoteControls; NSMutableArray *items; diff --git a/Classes/Controllers/PBGitWindowController.h b/Classes/Controllers/PBGitWindowController.h index b16f04ea5..1a1b65443 100644 --- a/Classes/Controllers/PBGitWindowController.h +++ b/Classes/Controllers/PBGitWindowController.h @@ -15,18 +15,15 @@ PBViewController *contentController; PBGitSidebarController *sidebarController; - IBOutlet NSView *sourceListControlsView; - IBOutlet NSSplitView *splitView; - IBOutlet NSView *sourceSplitView; - IBOutlet NSView *contentSplitView; + __weak IBOutlet NSView *sourceListControlsView; + __weak IBOutlet NSSplitView *splitView; + __weak IBOutlet NSView *sourceSplitView; + __weak IBOutlet NSView *contentSplitView; - IBOutlet NSTextField *statusField; - IBOutlet NSProgressIndicator *progressIndicator; + __weak IBOutlet NSTextField *statusField; + __weak IBOutlet NSProgressIndicator *progressIndicator; PBViewController* viewController; - - IBOutlet NSToolbarItem *terminalItem; - IBOutlet NSToolbarItem *finderItem; } @property (nonatomic, weak) PBGitRepository *repository; diff --git a/Classes/Controllers/PBGitWindowController.m b/Classes/Controllers/PBGitWindowController.m index b3c9b82bc..698133a39 100644 --- a/Classes/Controllers/PBGitWindowController.m +++ b/Classes/Controllers/PBGitWindowController.m @@ -86,12 +86,6 @@ - (void) awakeFromNib [[statusField cell] setBackgroundStyle:NSBackgroundStyleRaised]; [progressIndicator setUsesThreadedAnimation:YES]; - NSImage *finderImage = [[NSWorkspace sharedWorkspace] iconForFileType:NSFileTypeForHFSTypeCode(kFinderIcon)]; - [finderItem setImage:finderImage]; - - NSImage *terminalImage = [[NSWorkspace sharedWorkspace] iconForFile:@"/Applications/Utilities/Terminal.app/"]; - [terminalItem setImage:terminalImage]; - [self showWindow:nil]; } diff --git a/Classes/Controllers/PBHistorySearchController.h b/Classes/Controllers/PBHistorySearchController.h index 1ddef87f3..f003a7584 100644 --- a/Classes/Controllers/PBHistorySearchController.h +++ b/Classes/Controllers/PBHistorySearchController.h @@ -28,13 +28,13 @@ typedef enum historySearchModes { NSPanel *rewindPanel; } -@property (assign) IBOutlet PBGitHistoryController *historyController; -@property (assign) IBOutlet NSArrayController *commitController; +@property (weak) IBOutlet PBGitHistoryController *historyController; +@property (weak) IBOutlet NSArrayController *commitController; -@property (assign) IBOutlet NSSearchField *searchField; -@property (assign) IBOutlet NSSegmentedControl *stepper; -@property (assign) IBOutlet NSTextField *numberOfMatchesField; -@property (assign) IBOutlet NSProgressIndicator *progressIndicator; +@property (weak) IBOutlet NSSearchField *searchField; +@property (weak) IBOutlet NSSegmentedControl *stepper; +@property (weak) IBOutlet NSTextField *numberOfMatchesField; +@property (weak) IBOutlet NSProgressIndicator *progressIndicator; @property PBHistorySearchMode searchMode; diff --git a/Classes/Controllers/PBRefController.h b/Classes/Controllers/PBRefController.h index a9c11f4c2..62f115d87 100644 --- a/Classes/Controllers/PBRefController.h +++ b/Classes/Controllers/PBRefController.h @@ -16,11 +16,9 @@ @class PBRefMenuItem; @interface PBRefController : NSObject { - IBOutlet PBGitHistoryController *historyController; - IBOutlet NSArrayController *commitController; - IBOutlet PBCommitList *commitList; - - IBOutlet NSPopUpButton *branchPopUp; + __weak IBOutlet PBGitHistoryController *historyController; + __weak IBOutlet NSArrayController *commitController; + __weak IBOutlet PBCommitList *commitList; } - (void) fetchRemote:(PBRefMenuItem *)sender; diff --git a/Classes/Controllers/PBWebController.h b/Classes/Controllers/PBWebController.h index 543800c9e..b9c8a8d72 100644 --- a/Classes/Controllers/PBWebController.h +++ b/Classes/Controllers/PBWebController.h @@ -10,7 +10,7 @@ #import @interface PBWebController : NSObject { - IBOutlet WebView* view; + __weak IBOutlet WebView* view; NSString *startFile; BOOL finishedLoading; @@ -18,11 +18,11 @@ NSMapTable *callbacks; // For the repository access - IBOutlet id repository; + __weak IBOutlet id repository; } @property NSString *startFile; -@property id repository; +@property (weak) id repository; - (WebScriptObject *) script; - (void) closeView; diff --git a/Classes/Controllers/PBWebHistoryController.h b/Classes/Controllers/PBWebHistoryController.h index 9ddaad40c..f7e2816ea 100644 --- a/Classes/Controllers/PBWebHistoryController.h +++ b/Classes/Controllers/PBWebHistoryController.h @@ -18,8 +18,8 @@ @interface PBWebHistoryController : PBWebController { - IBOutlet PBGitHistoryController* historyController; - IBOutlet id contextMenuDelegate; + __weak IBOutlet PBGitHistoryController* historyController; + __weak IBOutlet id contextMenuDelegate; GTOID* currentSha; NSString* diff; diff --git a/Classes/PBCommitList.h b/Classes/PBCommitList.h index 60d528e11..4dcd9f9a6 100644 --- a/Classes/PBCommitList.h +++ b/Classes/PBCommitList.h @@ -16,10 +16,10 @@ typedef void(^PBFindPanelActionBlock)(id sender); @interface PBCommitList : NSTableView { - IBOutlet WebView* webView; - IBOutlet PBWebHistoryController *webController; - IBOutlet PBGitHistoryController *controller; - IBOutlet PBHistorySearchController *searchController; + __weak IBOutlet WebView* webView; + __weak IBOutlet PBWebHistoryController *webController; + __weak IBOutlet PBGitHistoryController *controller; + __weak IBOutlet PBHistorySearchController *searchController; BOOL useAdjustScroll; NSPoint mouseDownPoint; diff --git a/Classes/Views/GLFileView.h b/Classes/Views/GLFileView.h index 2d49d95ed..eecd77fdd 100644 --- a/Classes/Views/GLFileView.h +++ b/Classes/Views/GLFileView.h @@ -14,12 +14,12 @@ @class PBGitHistoryController; @interface GLFileView : PBWebController { - IBOutlet PBGitHistoryController* historyController; - IBOutlet MGScopeBar *typeBar; + __weak IBOutlet PBGitHistoryController* historyController; + __weak IBOutlet MGScopeBar *typeBar; NSMutableArray *groups; NSString *logFormat; - IBOutlet NSView *accessoryView; - IBOutlet NSSplitView *fileListSplitView; + __weak IBOutlet NSView *accessoryView; + __weak IBOutlet NSSplitView *fileListSplitView; } - (void)showFile; diff --git a/Classes/Views/PBGitRevisionCell.h b/Classes/Views/PBGitRevisionCell.h index 8c9fd9532..9c1b37037 100644 --- a/Classes/Views/PBGitRevisionCell.h +++ b/Classes/Views/PBGitRevisionCell.h @@ -16,8 +16,8 @@ PBGitCommit *objectValue; PBGraphCellInfo *cellInfo; NSTextFieldCell *textCell; - IBOutlet PBGitHistoryController *controller; - IBOutlet id contextMenuDelegate; + __weak IBOutlet PBGitHistoryController *controller; + __weak IBOutlet id contextMenuDelegate; } - (int) indexAtX:(float)x; diff --git a/Classes/Views/PBQLOutlineView.h b/Classes/Views/PBQLOutlineView.h index d0f8284ba..762503f3a 100644 --- a/Classes/Views/PBQLOutlineView.h +++ b/Classes/Views/PBQLOutlineView.h @@ -10,7 +10,7 @@ #import "PBGitHistoryController.h" @interface PBQLOutlineView : NSOutlineView { - IBOutlet PBGitHistoryController* controller; + __weak IBOutlet PBGitHistoryController* controller; } @end diff --git a/Classes/Views/PBQLTextView.h b/Classes/Views/PBQLTextView.h index 715e74efe..5b93b1751 100644 --- a/Classes/Views/PBQLTextView.h +++ b/Classes/Views/PBQLTextView.h @@ -13,7 +13,7 @@ @interface PBQLTextView : NSTextView { - IBOutlet PBGitHistoryController *controller; + __weak IBOutlet PBGitHistoryController *controller; } @end