From 499a93e32e4cc8d3a76138b2e43402d56b6716a2 Mon Sep 17 00:00:00 2001 From: Pietro Gagliardi Date: Thu, 7 Jan 2016 14:41:20 -0500 Subject: [PATCH] Completely rewrote the OS X map system to use NSMapTable correctly and hide the details behind more wrapper functions to prevent further mass rewrites; this fixes most of the crashing issues on OS X. The one with package ui still stands... --- darwin/button.m | 8 +++----- darwin/checkbox.m | 8 +++----- darwin/combobox.m | 8 +++----- darwin/entry.m | 8 +++----- darwin/map.m | 43 ++++++++++++++++++++++++++++-------------- darwin/menu.m | 2 +- darwin/slider.m | 8 +++----- darwin/uipriv_darwin.h | 10 ++++++---- darwin/window.m | 17 +++++++---------- 9 files changed, 58 insertions(+), 54 deletions(-) diff --git a/darwin/button.m b/darwin/button.m index 026cebf4..31e5c408 100644 --- a/darwin/button.m +++ b/darwin/button.m @@ -9,7 +9,7 @@ struct uiButton { }; @interface buttonDelegateClass : NSObject { - NSMapTable *buttons; + struct mapTable *buttons; } - (IBAction)onClicked:(id)sender; - (void)registerButton:(uiButton *)b; @@ -28,9 +28,7 @@ struct uiButton { - (void)dealloc { - if ([self->buttons count] != 0) - complain("attempt to destroy shared button delegate but buttons are still registered to it"); - [self->buttons release]; + mapDestroy(self->buttons); [super dealloc]; } @@ -52,7 +50,7 @@ struct uiButton { - (void)unregisterButton:(uiButton *)b { [b->button setTarget:nil]; - [self->buttons removeObjectForKey:b->button]; + mapDelete(self->buttons, b->button); } @end diff --git a/darwin/checkbox.m b/darwin/checkbox.m index 09af33a7..729ad5b3 100644 --- a/darwin/checkbox.m +++ b/darwin/checkbox.m @@ -9,7 +9,7 @@ struct uiCheckbox { }; @interface checkboxDelegateClass : NSObject { - NSMapTable *buttons; + struct mapTable *buttons; } - (IBAction)onToggled:(id)sender; - (void)registerCheckbox:(uiCheckbox *)c; @@ -28,9 +28,7 @@ struct uiCheckbox { - (void)dealloc { - if ([self->buttons count] != 0) - complain("attempt to destroy shared checkbox delegate but checkboxes are still registered to it"); - [self->buttons release]; + mapDestroy(self->buttons); [super dealloc]; } @@ -52,7 +50,7 @@ struct uiCheckbox { - (void)unregisterCheckbox:(uiCheckbox *)c { [c->button setTarget:nil]; - [self->buttons removeObjectForKey:c->button]; + mapDelete(self->buttons, c->button); } @end diff --git a/darwin/combobox.m b/darwin/combobox.m index 6789c834..27c32c08 100644 --- a/darwin/combobox.m +++ b/darwin/combobox.m @@ -33,7 +33,7 @@ struct uiCombobox { }; @interface comboboxDelegateClass : NSObject { - NSMapTable *comboboxes; + struct mapTable *comboboxes; } - (void)comboBoxSelectionDidChange:(NSNotification *)note; - (IBAction)onSelected:(id)sender; @@ -53,9 +53,7 @@ struct uiCombobox { - (void)dealloc { - if ([self->comboboxes count] != 0) - complain("attempt to destroy shared combobox delegate but comboboxes are still registered to it"); - [self->comboboxes release]; + mapDestroy(self->comboboxes); [super dealloc]; } @@ -93,7 +91,7 @@ struct uiCombobox { [c->cb setDelegate:nil]; else [c->pb setTarget:nil]; - [self->comboboxes removeObjectForKey:c->handle]; + mapDelete(self->comboboxes, c->handle); } @end diff --git a/darwin/entry.m b/darwin/entry.m index c9b5cfc1..d4ac5afb 100644 --- a/darwin/entry.m +++ b/darwin/entry.m @@ -28,7 +28,7 @@ struct uiEntry { }; @interface entryDelegateClass : NSObject { - NSMapTable *entries; + struct mapTable *entries; } - (void)controlTextDidChange:(NSNotification *)note; - (void)registerEntry:(uiEntry *)e; @@ -47,9 +47,7 @@ struct uiEntry { - (void)dealloc { - if ([self->entries count] != 0) - complain("attempt to destroy shared entry delegate but entries are still registered to it"); - [self->entries release]; + mapDestroy(self->entries); [super dealloc]; } @@ -70,7 +68,7 @@ struct uiEntry { - (void)unregisterEntry:(uiEntry *)e { [e->textfield setDelegate:nil]; - [self->entries removeObjectForKey:e->textfield]; + mapDelete(self->entries, e->textfield); } @end diff --git a/darwin/map.m b/darwin/map.m index 899865ca..968a86f9 100644 --- a/darwin/map.m +++ b/darwin/map.m @@ -1,29 +1,44 @@ // 17 august 2015 #import "uipriv_darwin.h" -// TODO are the NSValues (or worse, the NSMapTable) being garbage collected underfoot? open menuless window and check the checkbox; crash the package ui test when closing the main window; etc. +// TODO are the NSValues (or worse, the NSMapTable) being garbage collected underfoot? crash the package ui test when closing the main window; etc. // unfortunately NSMutableDictionary copies its keys, meaning we can't use it for pointers // hence, this file +// we could expose a NSMapTable directly, but let's treat all pointers as opaque and hide the implementation, just to be safe and prevent even more rewrites later +struct mapTable { + NSMapTable *m; +}; -NSMapTable *newMap(void) +struct mapTable *newMap(void) { - return [NSMapTable mapTableWithKeyOptions:(NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality) - valueOptions:NSPointerFunctionsOpaqueMemory]; + struct mapTable *m; + + m = uiNew(struct mapTable); + m->m = [NSMapTable mapTableWithKeyOptions:(NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality) + valueOptions:(NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality)]; + return m; } -void *mapGet(NSMapTable *map, id key) +void mapDestroy(struct mapTable *m) { - NSValue *v; - - v = (NSValue *) [map objectForKey:key]; - return [v pointerValue]; + if ([m->m count] != 0) + complain("attempt to destroy map with items inside; did you forget to deallocate something?"); + [m->m release]; + uiFree(m); } -void mapSet(NSMapTable *map, id key, void *value) +void *mapGet(struct mapTable *m, void *key) { - NSValue *v; - - v = [NSValue valueWithPointer:value]; - [map setObject:v forKey:key]; + return NSMapGet(m->m, key); +} + +void mapSet(struct mapTable *m, void *key, void *value) +{ + NSMapInsert(m->m, key, value); +} + +void mapDelete(struct mapTable *m, void *key) +{ + NSMapRemove(m->m, key); } diff --git a/darwin/menu.m b/darwin/menu.m index 6b08a0d5..14895c06 100644 --- a/darwin/menu.m +++ b/darwin/menu.m @@ -45,7 +45,7 @@ enum { - (void)dealloc { - [self->items release]; + mapDestroy(self->items); [super dealloc]; } diff --git a/darwin/slider.m b/darwin/slider.m index 9a04d22e..18906c5e 100644 --- a/darwin/slider.m +++ b/darwin/slider.m @@ -29,7 +29,7 @@ struct uiSlider { }; @interface sliderDelegateClass : NSObject { - NSMapTable *sliders; + struct mapTable *sliders; } - (IBAction)onChanged:(id)sender; - (void)registerSlider:(uiSlider *)b; @@ -48,9 +48,7 @@ struct uiSlider { - (void)dealloc { - if ([self->sliders count] != 0) - complain("attempt to destroy shared slider delegate but sliders are still registered to it"); - [self->sliders release]; + mapDestroy(self->sliders); [super dealloc]; } @@ -72,7 +70,7 @@ struct uiSlider { - (void)unregisterSlider:(uiSlider *)s { [s->slider setTarget:nil]; - [self->sliders removeObjectForKey:s->slider]; + mapDelete(self->sliders, s->slider); } @end diff --git a/darwin/uipriv_darwin.h b/darwin/uipriv_darwin.h index 6ee03c5e..bb784d32 100644 --- a/darwin/uipriv_darwin.h +++ b/darwin/uipriv_darwin.h @@ -15,7 +15,7 @@ // menu.m @interface menuManager : NSObject { - NSMapTable *items; + struct mapTable *items; BOOL hasQuit; BOOL hasPreferences; BOOL hasAbout; @@ -64,9 +64,11 @@ extern void layoutSingleView(NSView *, NSView *, int); extern NSSize fittingAlignmentSize(NSView *); // map.m -extern NSMapTable *newMap(void); -extern void *mapGet(NSMapTable *map, id key); -extern void mapSet(NSMapTable *map, id key, void *value); +extern struct mapTable *newMap(void); +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); // area.m extern int sendAreaEvents(NSEvent *); diff --git a/darwin/window.m b/darwin/window.m index 5ea3cb99..fda55e44 100644 --- a/darwin/window.m +++ b/darwin/window.m @@ -11,7 +11,7 @@ struct uiWindow { }; @interface windowDelegateClass : NSObject { - NSMapTable *windows; + struct mapTable *windows; } - (BOOL)windowShouldClose:(id)sender; - (void)registerWindow:(uiWindow *)w; @@ -31,9 +31,7 @@ struct uiWindow { - (void)dealloc { - if ([self->windows count] != 0) - complain("attempt to destroy shared window delegate but windows are still registered to it"); - [self->windows release]; + mapDestroy(self->windows); [super dealloc]; } @@ -57,17 +55,16 @@ struct uiWindow { - (void)unregisterWindow:(uiWindow *)w { [w->window setDelegate:nil]; - [self->windows removeObjectForKey:w->window]; + mapDelete(self->windows, w->window); } - (uiWindow *)lookupWindow:(NSWindow *)w { - NSValue *v; + uiWindow *v; - v = (NSValue *) [self->windows objectForKey:w]; - if (v == nil) // just in case we're called with some OS X-provided window as the key window - return NULL; - return (uiWindow *) [v pointerValue]; + v = (uiWindow *) mapGet(self->windows, w); + // this CAN (and IS ALLOWED TO) return NULL, just in case we're called with some OS X-provided window as the key window + return v; } @end