From 9353cc43b7c2e246da26fccdd30f3ea189056941 Mon Sep 17 00:00:00 2001 From: Cesar Celis Date: Fri, 24 Jul 2026 22:25:48 -0400 Subject: [PATCH 1/3] Balance the login item lookup, fixing an over-release applicationItemInList: returns from inside its loop, which skips the CFRelease of the snapshot array, so every lookup that finds a match leaks it. The same early return hands back an item it never retained, while both callers release it. The item's only reference is the array's, so that release does free it; the leak is what keeps the mistake invisible, because the orphaned array is never traversed again. Taking a reference before breaking out and releasing the snapshot on every path makes the two existing CFRelease calls legal, and CF_RETURNS_RETAINED puts the contract somewhere the compiler checks. Also releases the item handed back by LSSharedFileListInsertItemURL, which is declared CF_RETURNS_RETAINED and was being discarded, and guards the snapshot release against a NULL return. Measured with the leaks tool over 300 lookups that match: before, 300 root NSArray leaks totalling 2,347,440 bytes; after, 0 leaks for 0 total leaked bytes. The per-call cost scales with the size of the login items list, so the byte figure is specific to the machine it was measured on. --- AppDelegate.m | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/AppDelegate.m b/AppDelegate.m index bcde406..36a7e4f 100755 --- a/AppDelegate.m +++ b/AppDelegate.m @@ -170,22 +170,28 @@ - (BOOL)screensaverIsRunning { return activeAppID && [bundleIDs containsObject:activeAppID]; } -- (LSSharedFileListItemRef)applicationItemInList:(LSSharedFileListRef)list { +// Hands back a +1 reference, which is what both callers already release. +// CF_RETURNS_RETAINED states that so the compiler checks it, since the name +// does not follow the copy/create convention. +- (LSSharedFileListItemRef)applicationItemInList:(LSSharedFileListRef)list CF_RETURNS_RETAINED { NSString *appPath = [[NSBundle mainBundle] bundlePath]; - + NSArray *items = (id)LSSharedFileListCopySnapshot(list, NULL); - for(id item in items) { + LSSharedFileListItemRef found = NULL; + for(id item in items) { LSSharedFileListItemRef itemRef = (LSSharedFileListItemRef)item; CFURLRef URL = NULL; if(LSSharedFileListItemResolve(itemRef, 0, &URL, NULL)) continue; - + BOOL matches = [[(NSURL*)URL path] isEqual:appPath]; CFRelease(URL); - if(matches) - return itemRef; + if(matches) { + found = (LSSharedFileListItemRef)CFRetain(itemRef); + break; + } } - CFRelease(items); - return NULL; + if(items) CFRelease(items); + return found; } - (BOOL)startsAtLogin { @@ -205,7 +211,10 @@ - (void)setStartsAtLogin:(BOOL)start { if(start) { NSString *appPath = [[NSBundle mainBundle] bundlePath]; CFURLRef appURL = CFURLCreateWithFileSystemPath(NULL, (CFStringRef)appPath, kCFURLPOSIXPathStyle, YES); - LSSharedFileListInsertItemURL(loginItems, kLSSharedFileListItemLast, NULL, NULL, appURL, NULL, NULL); + // Declared CF_RETURNS_RETAINED in LSSharedFileList.h, so the item it + // hands back has to be released too. + LSSharedFileListItemRef inserted = LSSharedFileListInsertItemURL(loginItems, kLSSharedFileListItemLast, NULL, NULL, appURL, NULL, NULL); + if(inserted) CFRelease(inserted); CFRelease(appURL); }else{ LSSharedFileListItemRef item = [self applicationItemInList:loginItems]; From 8196a992d8c543f941f6ccc3fee39bce7b0d42c9 Mon Sep 17 00:00:00 2001 From: Cesar Celis Date: Fri, 24 Jul 2026 22:26:14 -0400 Subject: [PATCH 2/3] Reuse the Donate and Send Feedback web views launchDonate: and launchFeedback: each allocated a WKWebView on every invocation and added it on top of the one already in the window. Nothing removed the old ones, so a window ends up holding one web view per visit, each with its own WebContent process behind it. Opening the two windows in a loop, counting subviews in the feedback window: it starts at zero, gains exactly one per round on master, and stays at one here. The memory cost is easier to see system wide than in the app: at 300 rounds master left 404 WebContent processes alive. Resident memory of the app itself grows too, though the figure moves a lot depending on how long each page is given to settle, so the subview and process counts are the reliable part. The content view owns the web view, so this adds no retain of its own, and the frame is refreshed on reuse in case those windows ever become resizable. --- AppDelegate.m | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/AppDelegate.m b/AppDelegate.m index 36a7e4f..d43a7fd 100755 --- a/AppDelegate.m +++ b/AppDelegate.m @@ -311,6 +311,27 @@ - (IBAction)showPreferences:(id)sender { # pragma mark - Help & Feedback Window Utility Methods +// Each open used to allocate another WKWebView and stack it on the one already +// there, so a window accumulated a full web view, and its WebContent process, +// per visit. Reuse the one the window already holds. +// +// The window's content view owns the web view, so nothing here retains it. Both +// of these windows are releasedWhenClosed="NO" in the nib, so it survives being +// closed. The frame is refreshed on reuse in case the window ever becomes +// resizable. +- (WKWebView *)webViewForWindow:(NSWindow *)window { + NSView *contentView = [window contentView]; + for(NSView *subview in [contentView subviews]) { + if([subview isKindOfClass:[WKWebView class]]) { + [subview setFrame:[contentView frame]]; + return (WKWebView *)subview; + } + } + WKWebView *webView = [[[WKWebView alloc] initWithFrame:[contentView frame]] autorelease]; + [contentView addSubview:webView]; + return webView; +} + -(IBAction)launchHelpCenter:(id)sender { [helpCenterWindow center]; [helpCenterWindow setIsVisible:YES]; @@ -331,9 +352,7 @@ -(IBAction)launchFeedback:(id)sender { NSURL *nsurl=[NSURL URLWithString:[NSString stringWithFormat:@"%@%@", webBaseURL, @"/feedback"]]; if (NSClassFromString(@"WKWebView")) { NSURLRequest *nsrequest=[NSURLRequest requestWithURL:nsurl]; - WKWebView *feedbackWebView = [[WKWebView alloc] initWithFrame:[[feedbackWindow contentView] frame]]; - [feedbackWebView loadRequest:nsrequest]; - [[feedbackWindow contentView] addSubview:feedbackWebView]; + [[self webViewForWindow:feedbackWindow] loadRequest:nsrequest]; [feedbackWindow center]; [feedbackWindow setIsVisible:YES]; [feedbackWindow makeKeyAndOrderFront:nil]; @@ -346,9 +365,7 @@ -(IBAction)launchDonate:(id)sender { NSURL *nsurl=[NSURL URLWithString:[NSString stringWithFormat:@"%@%@", webBaseURL, @"/donate"]]; if (NSClassFromString(@"WKWebView")) { NSURLRequest *nsrequest=[NSURLRequest requestWithURL:nsurl]; - WKWebView *donateWebView = [[WKWebView alloc] initWithFrame:[[donateWindow contentView] frame]]; - [donateWebView loadRequest:nsrequest]; - [[donateWindow contentView] addSubview:donateWebView]; + [[self webViewForWindow:donateWindow] loadRequest:nsrequest]; [donateWindow center]; [donateWindow setIsVisible:YES]; [donateWindow makeKeyAndOrderFront:nil]; From 730852282f59616dbd41365cfcf37bb4115d1f3a Mon Sep 17 00:00:00 2001 From: Cesar Celis Date: Fri, 24 Jul 2026 22:26:53 -0400 Subject: [PATCH 3/3] Satisfy the remaining analyzer findings, defensively These are hygiene, not fixes for anything observable, and the comments say so in the files rather than only here. Neither dealloc runs today. AppDelegate is kept alive by the repeating timer it schedules on the run loop, and LCMenuIconView by the status item that setView: makes retain it. Both objects are meant to live for the life of the process. Writing correct teardown anyway is what osx.cocoa.Dealloc asks for, and it means the code is right if either lifetime ever changes. The two initializers used their instance variables without assigning the result of the super call. LCMenuIconView had the same defect three lines from the one the analyzer flagged in AppDelegate. With this, xcodebuild analyze goes from 6 findings to 0. --- AppDelegate.m | 10 +++++++++- LCMenuIconView.m | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/AppDelegate.m b/AppDelegate.m index d43a7fd..3a8aec2 100755 --- a/AppDelegate.m +++ b/AppDelegate.m @@ -15,7 +15,8 @@ @implementation AppDelegate # pragma mark - Initialization - (id)init { - [super init]; + self = [super init]; + if(!self) return nil; timer = [[NSTimer scheduledTimerWithTimeInterval:10.0 target:self selector:@selector(timer:) userInfo:nil repeats:YES] retain]; webBaseURL = @"https://www.intelliscapesolutions.com/apps/caffeine"; @@ -380,10 +381,17 @@ -(IBAction)showProblemReportInfoPopoverButton:(id)sender { # pragma mark - Maintenance & Memory Management +// Note this does not currently run. init schedules a repeating timer whose +// target is self, and the run loop holds that timer, so retainCount never +// reaches zero. Measured: 2 right after init, 1 after releasing the only +// reference the app holds. Correct teardown regardless, and it is what +// osx.cocoa.Dealloc is asking for. - (void)dealloc { + [[[NSWorkspace sharedWorkspace] notificationCenter] removeObserver:self]; [timer invalidate]; [timer release]; [menuView release]; + [timeoutTimer invalidate]; [timeoutTimer release]; [super dealloc]; } diff --git a/LCMenuIconView.m b/LCMenuIconView.m index ad09d49..09a9ae5 100755 --- a/LCMenuIconView.m +++ b/LCMenuIconView.m @@ -12,6 +12,7 @@ @implementation LCMenuIconView - (id)initWithFrame:(NSRect)r { self = [super initWithFrame:r]; + if(!self) return nil; statusItem = [[[NSStatusBar systemStatusBar] statusItemWithLength:30] retain]; [statusItem setVisible:YES]; [statusItem setView:self]; @@ -124,4 +125,27 @@ - (void)observeValueForKeyPath:(NSString *)keyPath [self setNeedsDisplay]; } +# pragma mark - Memory Management + +// Note this does not currently run either. setView: makes the status item +// retain this view while the view retains the status item, and the status item +// is held by the system status bar on top of that; retainCount right after +// initWithFrame: measures 9, and releasing the app's only reference does not +// deallocate. Exactly one of these is created, in awakeFromNib, and it lives +// for the life of the process, which for a menu bar icon is the intended +// behavior rather than a leak. +// +// It is here because the class holds retained ivars and osx.cocoa.Dealloc is +// right to ask for a correct teardown, not because it fixes an observed fault. +- (void)dealloc { + if (@available(macOS 10.14, *)) { + [statusItem removeObserver:self forKeyPath:@"view.effectiveAppearance"]; + }else{ + [[NSUserDefaults standardUserDefaults] removeObserver:self forKeyPath:@"AppleInterfaceStyle"]; + } + [statusItem release]; + [menu release]; + [super dealloc]; +} + @end