aboutsummaryrefslogtreecommitdiffstats
path: root/Source
diff options
context:
space:
mode:
authorMax <dmoagx@users.noreply.github.com>2018-05-17 11:00:41 +0200
committerMax <dmoagx@users.noreply.github.com>2018-05-17 11:00:41 +0200
commitf20ab36802c5b2733fc1cb1900f60df08ea8c1d3 (patch)
treec8f6cf04a1b8b558cc4f73366108c4e988ed21cf /Source
parentc7efb528aea160aed4dbd32e2424ad5959f94550 (diff)
downloadsequelpro-f20ab36802c5b2733fc1cb1900f60df08ea8c1d3.tar.gz
sequelpro-f20ab36802c5b2733fc1cb1900f60df08ea8c1d3.tar.bz2
sequelpro-f20ab36802c5b2733fc1cb1900f60df08ea8c1d3.zip
#63: Fix a crash when closing a tab
Diffstat (limited to 'Source')
-rw-r--r--Source/SPRuleFilterController.h2
-rw-r--r--Source/SPRuleFilterController.m113
2 files changed, 97 insertions, 18 deletions
diff --git a/Source/SPRuleFilterController.h b/Source/SPRuleFilterController.h
index 45c28fce..4d425329 100644
--- a/Source/SPRuleFilterController.h
+++ b/Source/SPRuleFilterController.h
@@ -47,7 +47,7 @@ NSString * const SPRuleFilterHeightChangedNotification;
NSMutableDictionary *contentFilters;
NSMutableDictionary *numberOfDefaultFilters;
- NSMutableArray *model;
+ id _modelContainer; // private class
SPContentFilterManager *contentFilterManager;
diff --git a/Source/SPRuleFilterController.m b/Source/SPRuleFilterController.m
index baba3eb7..104e1cbc 100644
--- a/Source/SPRuleFilterController.m
+++ b/Source/SPRuleFilterController.m
@@ -164,13 +164,47 @@ const NSString * const SerFilterExprDefinition = @"_filterDefinition";
#pragma mark -
+/**
+ * TODO:
+ * This class shouldn't even exist to begin with.
+ * Its sad story begins with this call in `-[SPRuleFilterController dealloc]`:
+ *
+ * [filterRuleEditor unbind:@"rows"];
+ *
+ * `-dealloc` may not be the best method to undo what we did in `-awakeFromNib`, but it's the only thing we have.
+ * Also we have to unbind this object, or we may receive zombie calls later on because the binding is unretained.
+ * Which brings us to another huge mistake Apple made in the implementation of -unbind. The call looks like this:
+ *
+ * - [NSRulEditor unbind:]
+ * - [NSRuleEditor _rootRowsArray]
+ * - [NSRuleEditor->_boundArrayOwner mutableArrayValueForKeyPath:NSRuleEditor->_boundArrayKeyPath]
+ *
+ * -mutableArrayValueForKeyPath: is the culprit here since it does not return the object itself ("model") but
+ * instead returns an autoreleased proxy object which retains the parent object of the key.
+ *
+ * That explains why we can't put "model" into SPRuleFilterController:
+ * The `-[NSRuleEditor unbind:]` would cause a call to `-[SPRuleFilterController retain]` from within
+ * `-[SPRuleFilterController dealloc]` (which is pointless since there is no way out from -dealloc).
+ * This wouldn't be a problem if the proxy object was released again while dealloc is still on the stack, but
+ * since it is autoreleased we end up with a zombie call again.
+ *
+ * ModelContainer is a dummy intermediate to prevent this, since it is still valid when we enter -dealloc and
+ * trigger -unbind and thus can handle the -retain by the proxy object.
+ */
+@interface ModelContainer : NSObject
+{
+ NSMutableArray *model;
+}
+// This is the binding used by NSRuleEditor for the current state
+@property (retain, nonatomic) NSMutableArray *model;
+@end
+
+#pragma mark -
+
@interface SPRuleFilterController () <NSRuleEditorDelegate>
@property (readwrite, assign, nonatomic) CGFloat preferredHeight;
-// This is the binding used by NSRuleEditor for the current state
-@property (retain, nonatomic) NSMutableArray *model;
-
- (NSArray *)_compareTypesForColumn:(ColumnNode *)colNode;
- (IBAction)_textFieldAction:(id)sender;
- (IBAction)_editFiltersAction:(id)sender;
@@ -192,7 +226,6 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
@implementation SPRuleFilterController
-@synthesize model = model;
@synthesize preferredHeight = preferredHeight;
@synthesize target = target;
@synthesize action = action;
@@ -201,7 +234,7 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
{
if((self = [super init])) {
columns = [[NSMutableArray alloc] init];
- model = [[NSMutableArray alloc] init];
+ _modelContainer = [[ModelContainer alloc] init];
preferredHeight = 0.0;
target = nil;
action = NULL;
@@ -243,7 +276,7 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (void)awakeFromNib
{
- [filterRuleEditor bind:@"rows" toObject:self withKeyPath:@"model" options:nil];
+ [filterRuleEditor bind:@"rows" toObject:_modelContainer withKeyPath:@"model" options:nil];
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(_contentFiltersHaveBeenUpdated:)
@@ -253,7 +286,7 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (void)focusFirstInputField
{
- for(NSDictionary *rootItem in model) {
+ for(NSDictionary *rootItem in [_modelContainer model]) {
if([self _focusOnFieldInSubtree:rootItem]) return;
}
}
@@ -279,9 +312,8 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (void)setColumns:(NSArray *)dataColumns;
{
- [self willChangeValueForKey:@"model"]; // manual KVO is needed for filter rule editor to notice change
- [model removeAllObjects];
- [self didChangeValueForKey:@"model"];
+ // we have to access the model in the same way the rule editor does for it to realize the changes
+ [[_modelContainer mutableArrayValueForKey:@"model"] removeAllObjects];
[columns removeAllObjects];
@@ -519,8 +551,10 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (void)dealloc
{
+ [filterRuleEditor unbind:@"rows"];
[[NSNotificationCenter defaultCenter] removeObserver:self];
- SPClear(model);
+ // WARNING: THIS MUST COME AFTER -unbind:! See the class comment on ModelContainer for the reasoning
+ SPClear(_modelContainer);
SPClear(columns);
SPClear(contentFilters);
SPClear(numberOfDefaultFilters);
@@ -736,7 +770,7 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (BOOL)isEmpty
{
- return ([[self model] count] == 0);
+ return ([[_modelContainer model] count] == 0);
}
- (void)addFilterExpression
@@ -797,8 +831,8 @@ static void _addIfNotNil(NSMutableArray *array, id toAdd);
- (NSDictionary *)_serializedFilterIncludingFilterDefinition:(BOOL)includeDefinition
{
- NSMutableArray *rootItems = [NSMutableArray arrayWithCapacity:[model count]];
- for(NSDictionary *item in model) {
+ NSMutableArray *rootItems = [NSMutableArray arrayWithCapacity:[[_modelContainer model] count]];
+ for(NSDictionary *item in [_modelContainer model]) {
[rootItems addObject:[self _serializeSubtree:item includingDefinition:includeDefinition]];
}
//the root serialized filter can either be an AND of multiple root items or a single root item
@@ -872,9 +906,7 @@ void _addIfNotNil(NSMutableArray *array, id toAdd)
{
if(!serialized) return;
- // we have to exchange the whole model object or NSRuleEditor will get confused
NSMutableArray *newModel = [[NSMutableArray alloc] init];
-
@autoreleasepool {
// if the root object is an AND group directly restore its contents, otherwise restore the object
if(SerIsGroup(serialized) && [[serialized objectForKey:SerFilterGroupIsConjunction] boolValue]) {
@@ -887,7 +919,10 @@ void _addIfNotNil(NSMutableArray *array, id toAdd)
}
}
- [self setModel:newModel];
+ // we have to access the model in the same way the rule editor does for it to realize the changes
+ NSMutableArray *proxy = [_modelContainer mutableArrayValueForKey:@"model"];
+ [proxy setArray:newModel];
+
[newModel release];
}
@@ -1353,3 +1388,47 @@ BOOL SerIsGroup(NSDictionary *dict)
}
@end
+
+#pragma mark -
+
+@implementation ModelContainer
+
+@synthesize model = model;
+
+- (instancetype)init
+{
+ if (self = [super init]) {
+ model = [[NSMutableArray alloc] init];
+ }
+ return self;
+}
+
+- (void)dealloc
+{
+ [self setModel:nil];
+ [super dealloc];
+}
+
+// KVO
+
+- (void)insertObject:(id)obj inModelAtIndex:(NSUInteger)idx
+{
+ [model insertObject:obj atIndex:idx];
+}
+
+- (void)removeObjectFromModelAtIndex:(NSUInteger)idx
+{
+ [model removeObjectAtIndex:idx];
+}
+
+- (void)insertModel:(NSArray *)array atIndexes:(NSIndexSet *)indexes
+{
+ [model insertObjects:array atIndexes:indexes];
+}
+
+- (void)removeModelAtIndexes:(NSIndexSet *)indexes
+{
+ [model removeObjectsAtIndexes:indexes];
+}
+
+@end