From 2ebb9052cc8eb4be99cad5df3ae15d586504bab0 Mon Sep 17 00:00:00 2001 From: Kevin Wojniak Date: Mon, 23 May 2016 21:09:46 -0700 Subject: [PATCH 1/3] Fix crash when closing program on OS X This fixes #14. Autorelease pools need to be used to make sure objects get properly released. Unfortunately this produces a new error when menuManager gets deallocated, which I am looking at fixing: map.m:25:mapDestroy() POSSIBLE IMPLEMENTATION BUG; CONTACT ANDLABS: attempt to destroy map with items inside --- darwin/main.m | 20 +++++++++++++++++--- darwin/menu.m | 30 +++++++++++++++++++----------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/darwin/main.m b/darwin/main.m index fa0e5782..7ea860c9 100644 --- a/darwin/main.m +++ b/darwin/main.m @@ -2,6 +2,7 @@ #import "uipriv_darwin.h" static BOOL canQuit = NO; +static NSAutoreleasePool *globalPool; @implementation applicationClass @@ -72,7 +73,8 @@ static BOOL canQuit = NO; - (void)dealloc { - [self.menuManager release]; + // Apple docs: "Don't Use Accessor Methods in Initializer Methods and dealloc" + [_menuManager release]; [super dealloc]; } @@ -99,6 +101,10 @@ uiInitOptions options; const char *uiInit(uiInitOptions *o) { + globalPool = [[NSAutoreleasePool alloc] init]; + + @autoreleasepool { + options = *o; [applicationClass sharedApplication]; // don't check for a NO return; something (launch services?) causes running from application bundles to always return NO when asking to change activation policy, even if the change is to the same activation policy! @@ -109,21 +115,29 @@ const char *uiInit(uiInitOptions *o) initAlloc(); // always do this so we always have an application menu - appDelegate().menuManager = [menuManager new]; + appDelegate().menuManager = [[menuManager new] autorelease]; [realNSApp() setMainMenu:[appDelegate().menuManager makeMenubar]]; setupFontPanel(); return NULL; + + } // @autoreleasepool } void uiUninit(void) { + @autoreleasepool { + uninitMenus(); + [appDelegate() release]; [realNSApp() setDelegate:nil]; - [appDelegate() release]; [realNSApp() release]; uninitAlloc(); + + } // @autoreleasepool + + [globalPool release]; } void uiFreeInitError(const char *err) diff --git a/darwin/menu.m b/darwin/menu.m index 0f33df1d..12ce5c26 100644 --- a/darwin/menu.m +++ b/darwin/menu.m @@ -116,14 +116,14 @@ enum { NSMenu *servicesMenu; appName = [[NSProcessInfo processInfo] processName]; - appMenuItem = [[NSMenuItem alloc] initWithTitle:appName action:NULL keyEquivalent:@""]; - appMenu = [[NSMenu alloc] initWithTitle:appName]; + appMenuItem = [[[NSMenuItem alloc] initWithTitle:appName action:NULL keyEquivalent:@""] autorelease]; + appMenu = [[[NSMenu alloc] initWithTitle:appName] autorelease]; [appMenuItem setSubmenu:appMenu]; [menubar addItem:appMenuItem]; // first is About title = [@"About " stringByAppendingString:appName]; - item = [[NSMenuItem alloc] initWithTitle:title action:@selector(onClicked:) keyEquivalent:@""]; + item = [[[NSMenuItem alloc] initWithTitle:title action:@selector(onClicked:) keyEquivalent:@""] autorelease]; [item setTarget:self]; [appMenu addItem:item]; self.aboutItem = item; @@ -131,7 +131,7 @@ enum { [appMenu addItem:[NSMenuItem separatorItem]]; // next is Preferences - item = [[NSMenuItem alloc] initWithTitle:@"Preferences…" action:@selector(onClicked:) keyEquivalent:@","]; + item = [[[NSMenuItem alloc] initWithTitle:@"Preferences…" action:@selector(onClicked:) keyEquivalent:@","] autorelease]; [item setTarget:self]; [appMenu addItem:item]; self.preferencesItem = item; @@ -139,8 +139,8 @@ enum { [appMenu addItem:[NSMenuItem separatorItem]]; // next is Services - item = [[NSMenuItem alloc] initWithTitle:@"Services" action:NULL keyEquivalent:@""]; - servicesMenu = [[NSMenu alloc] initWithTitle:@"Services"]; + item = [[[NSMenuItem alloc] initWithTitle:@"Services" action:NULL keyEquivalent:@""] autorelease]; + servicesMenu = [[[NSMenu alloc] initWithTitle:@"Services"] autorelease]; [item setSubmenu:servicesMenu]; [realNSApp() setServicesMenu:servicesMenu]; [appMenu addItem:item]; @@ -149,14 +149,14 @@ enum { // next are the three hiding options title = [@"Hide " stringByAppendingString:appName]; - item = [[NSMenuItem alloc] initWithTitle:title action:@selector(hide:) keyEquivalent:@"h"]; + item = [[[NSMenuItem alloc] initWithTitle:title action:@selector(hide:) keyEquivalent:@"h"] autorelease]; // the .xib file says they go to -1 ("First Responder", which sounds wrong...) // to do that, we simply leave the target as nil [appMenu addItem:item]; - item = [[NSMenuItem alloc] initWithTitle:@"Hide Others" action:@selector(hideOtherApplications:) keyEquivalent:@"h"]; + item = [[[NSMenuItem alloc] initWithTitle:@"Hide Others" action:@selector(hideOtherApplications:) keyEquivalent:@"h"] autorelease]; [item setKeyEquivalentModifierMask:(NSAlternateKeyMask | NSCommandKeyMask)]; [appMenu addItem:item]; - item = [[NSMenuItem alloc] initWithTitle:@"Show All" action:@selector(unhideAllApplications:) keyEquivalent:@""]; + item = [[[NSMenuItem alloc] initWithTitle:@"Show All" action:@selector(unhideAllApplications:) keyEquivalent:@""] autorelease]; [appMenu addItem:item]; [appMenu addItem:[NSMenuItem separatorItem]]; @@ -164,7 +164,7 @@ enum { // and finally Quit // DON'T use @selector(terminate:) as the action; we handle termination ourselves title = [@"Quit " stringByAppendingString:appName]; - item = [[NSMenuItem alloc] initWithTitle:title action:@selector(onQuitClicked:) keyEquivalent:@"q"]; + item = [[[NSMenuItem alloc] initWithTitle:title action:@selector(onQuitClicked:) keyEquivalent:@"q"] autorelease]; [item setTarget:self]; [appMenu addItem:item]; self.quitItem = item; @@ -174,7 +174,7 @@ enum { { NSMenu *menubar; - menubar = [[NSMenu alloc] initWithTitle:@""]; + menubar = [[[NSMenu alloc] initWithTitle:@""] autorelease]; [self buildApplicationMenu:menubar]; return menubar; } @@ -222,6 +222,8 @@ void uiMenuItemSetChecked(uiMenuItem *item, int checked) static uiMenuItem *newItem(uiMenu *m, int type, const char *name) { + @autoreleasepool { + uiMenuItem *item; if (menusFinalized) @@ -257,6 +259,8 @@ static uiMenuItem *newItem(uiMenu *m, int type, const char *name) [m->items addObject:[NSValue valueWithPointer:item]]; return item; + + } // @autoreleasepool } uiMenuItem *uiMenuAppendItem(uiMenu *m, const char *name) @@ -294,6 +298,8 @@ void uiMenuAppendSeparator(uiMenu *m) uiMenu *uiNewMenu(const char *name) { + @autoreleasepool { + uiMenu *m; if (menusFinalized) @@ -316,6 +322,8 @@ uiMenu *uiNewMenu(const char *name) [menus addObject:[NSValue valueWithPointer:m]]; return m; + + } // @autoreleasepool } void finalizeMenus(void) From abb3c39c7829c595b323fdf09353c50d8d0507db Mon Sep 17 00:00:00 2001 From: Kevin Wojniak Date: Mon, 23 May 2016 21:11:12 -0700 Subject: [PATCH 2/3] Spaces to tabs --- darwin/main.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/darwin/main.m b/darwin/main.m index 7ea860c9..15c91fea 100644 --- a/darwin/main.m +++ b/darwin/main.m @@ -130,7 +130,7 @@ void uiUninit(void) @autoreleasepool { uninitMenus(); - [appDelegate() release]; + [appDelegate() release]; [realNSApp() setDelegate:nil]; [realNSApp() release]; uninitAlloc(); From 49e17cbfd71f35281045dd68a87086fd280be310 Mon Sep 17 00:00:00 2001 From: Kevin Wojniak Date: Mon, 23 May 2016 21:41:52 -0700 Subject: [PATCH 3/3] Fix "attempt to destroy map with items inside" when menuManager is deallocated Fixes #58. The map needs to have its contents properly freed which requires releasing the properly retaining the NSMenuItem object. --- darwin/main.m | 1 - darwin/map.m | 16 ++++++++++++++++ darwin/menu.m | 20 +++++++++++++++----- darwin/uipriv_darwin.h | 2 ++ 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/darwin/main.m b/darwin/main.m index 15c91fea..e8985e99 100644 --- a/darwin/main.m +++ b/darwin/main.m @@ -129,7 +129,6 @@ void uiUninit(void) { @autoreleasepool { - uninitMenus(); [appDelegate() release]; [realNSApp() setDelegate:nil]; [realNSApp() release]; diff --git a/darwin/map.m b/darwin/map.m index 67b5edb6..46a7b8d2 100644 --- a/darwin/map.m +++ b/darwin/map.m @@ -41,3 +41,19 @@ void mapDelete(struct mapTable *m, void *key) { NSMapRemove(m->m, key); } + +void mapWalk(struct mapTable *m, void (*f)(void *key, void *value)) +{ + NSMapEnumerator e = NSEnumerateMapTable(m->m); + void *k = NULL; + void *v = NULL; + while (NSNextMapEnumeratorPair(&e, &k, &v)) { + f(k, v); + } + NSEndMapTableEnumeration(&e); +} + +void mapReset(struct mapTable *m) +{ + NSResetMapTable(m->m); +} diff --git a/darwin/menu.m b/darwin/menu.m index 12ce5c26..fcf819da 100644 --- a/darwin/menu.m +++ b/darwin/menu.m @@ -27,6 +27,14 @@ enum { typeSeparator, }; +static void mapItemReleaser(void *key, void *value) +{ + uiMenuItem *item; + + item = (uiMenuItem *)value; + [item->item release]; +} + @implementation menuManager - (id)init @@ -43,6 +51,9 @@ enum { - (void)dealloc { + uninitMenus(); + mapWalk(self->items, mapItemReleaser); + mapReset(self->items); mapDestroy(self->items); [super dealloc]; } @@ -234,16 +245,16 @@ static uiMenuItem *newItem(uiMenu *m, int type, const char *name) item->type = type; switch (item->type) { case typeQuit: - item->item = appDelegate().menuManager.quitItem; + item->item = [appDelegate().menuManager.quitItem retain]; break; case typePreferences: - item->item = appDelegate().menuManager.preferencesItem; + item->item = [appDelegate().menuManager.preferencesItem retain]; break; case typeAbout: - item->item = appDelegate().menuManager.aboutItem; + item->item = [appDelegate().menuManager.aboutItem retain]; break; case typeSeparator: - item->item = [NSMenuItem separatorItem]; + item->item = [[NSMenuItem separatorItem] retain]; [m->menu addItem:item->item]; break; default: @@ -335,7 +346,6 @@ void uninitMenus(void) { if (menus == NULL) return; - // don't worry about the actual NSMenus and NSMenuItems; they'll be freed when we clean up the NSApplication [menus enumerateObjectsUsingBlock:^(id obj, NSUInteger index, BOOL *stop) { NSValue *v; uiMenu *m; diff --git a/darwin/uipriv_darwin.h b/darwin/uipriv_darwin.h index 4a8b1bdd..865d3a48 100644 --- a/darwin/uipriv_darwin.h +++ b/darwin/uipriv_darwin.h @@ -89,6 +89,8 @@ extern void mapDestroy(struct mapTable *m); extern void *mapGet(struct mapTable *m, void *key); extern void mapSet(struct mapTable *m, void *key, void *value); extern void mapDelete(struct mapTable *m, void *key); +extern void mapWalk(struct mapTable *m, void (*f)(void *key, void *value)); +extern void mapReset(struct mapTable *m); // area.m extern int sendAreaEvents(NSEvent *);