Skip to content

Commit

Permalink
Fixed race condition in search plugins when canceling old search
Browse files Browse the repository at this point in the history
  • Loading branch information
griff committed May 11, 2010
1 parent 302ca0f commit 6573a8c
Show file tree
Hide file tree
Showing 20 changed files with 171 additions and 44 deletions.
22 changes: 22 additions & 0 deletions Framework/src/MZBaseSearchProvider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//
// MZBaseSearchProvider.h
// MetaZ
//
// Created by Brian Olsen on 11/05/10.
// Copyright 2010 Maven-Group. All rights reserved.
//

#import <Cocoa/Cocoa.h>


@interface MZBaseSearchProvider : NSObject
{
@private
id search;
NSMutableArray* canceledSearches;
}

- (void)cancelSearch;
- (void)startSearch:(id)search;

@end
88 changes: 88 additions & 0 deletions Framework/src/MZBaseSearchProvider.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//
// MZBaseSearchProvider.m
// MetaZ
//
// Created by Brian Olsen on 11/05/10.
// Copyright 2010 Maven-Group. All rights reserved.
//

#import "MZBaseSearchProvider.h"
#import "GTMNSObject+KeyValueObserving.h"

@implementation MZBaseSearchProvider

- (id)init
{
self = [super init];
if(self)
{
canceledSearches = [[NSMutableArray alloc] init];
}
return self;
}

- (void)dealloc
{
//[search gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(searchFinished:)];
for(id canceledSearch in canceledSearches)
[canceledSearch gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(canceledSearchFinished:)];

[search release];
[canceledSearches release];
[super dealloc];
}

- (void)cancelSearch
{
// Finish last search;
if(search)
{
@synchronized(search)
{
if(![search isFinished])
{
[canceledSearches addObject:search];
[search gtm_addObserver:self forKeyPath:@"isFinished" selector:@selector(canceledSearchFinished:) userInfo:nil options:0];
}
//[search gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(searchFinished:)];
}
[search cancel];
[search release];
search = nil;
}
}

- (void)startSearch:(id)theSearch
{
search = [theSearch retain];
//[search gtm_addObserver:self forKeyPath:@"isFinished" selector:@selector(searchFinished:) userInfo:nil options:0];
}

/*
- (void)searchFinishedMain
{
[search autorelease];
search = nil;
}
- (void)searchFinished:(GTMKeyValueChangeNotification *)notification
{
[search gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(searchFinished:)];
[self performSelectorOnMainThread:@selector(searchFinishedMain) withObject:nil waitUntilDone:YES];
}
*/

- (void)canceledSearchFinishedMain:(id)theSearch
{
[canceledSearches removeObject:theSearch];
}

- (void)canceledSearchFinished:(GTMKeyValueChangeNotification *)notification
{
[[notification object] gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(canceledSearchFinished:)];
[self performSelectorOnMainThread:@selector(canceledSearchFinishedMain:)
withObject:[notification object]
waitUntilDone:NO];
}

@end
2 changes: 1 addition & 1 deletion Framework/src/MZOperationsController.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
- (void)removeOperation:(NSOperation *)operation;

- (void)cancel;
- (void)waitUntilFinished;
//- (void)waitUntilFinished;
- (void)addOperationsToQueue:(NSOperationQueue*)queue;

- (void)operationsFinished;
Expand Down
4 changes: 3 additions & 1 deletion Framework/src/MZOperationsController.m
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ - (void)dealloc
for(NSOperation* op in operations)
{
if(![op isFinished])
MZLoggerInfo(@"Deallocing owner of operations");
MZLoggerError(@"Deallocing owner of operations");
//[op gtm_removeObserver:self forKeyPath:@"finished" selector:@selector(operationFinished:)];
[op gtm_removeObserver:self forKeyPath:@"isFinished" selector:@selector(operationFinished:)];
if([op isKindOfClass:[MZErrorOperation class]])
Expand Down Expand Up @@ -91,6 +91,7 @@ - (void)cancel
[op cancel];
}

/*
- (void)waitUntilFinished
{
if(![self isFinished])
Expand All @@ -100,6 +101,7 @@ - (void)waitUntilFinished
MZLoggerDebug(@"Fuck that");
}
}
*/

- (void)addOperationsToQueue:(NSOperationQueue*)queue
{
Expand Down
2 changes: 1 addition & 1 deletion Framework/src/MZRESTOperation.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
- (void)start;
- (BOOL)isConcurrent;
- (void)cancel;
- (void)waitUntilFinished;
//- (void)waitUntilFinished;

- (void)operationFinished;

Expand Down
2 changes: 2 additions & 0 deletions Framework/src/MZRESTOperation.m
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,13 @@ - (void)cancel
[wrapper cancelConnection];
}

/*
- (void)waitUntilFinished
{
if(![self isFinished])
[self waitForChangedKeyPath:@"finished"];
}
*/

- (void)operationFinished
{
Expand Down
1 change: 1 addition & 0 deletions Framework/src/MetaZKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#import <MetaZKit/MZTaskOperation.h>
#import <MetaZKit/MZOperationsController.h>
#import <MetaZKit/MZReadOperationsController.h>
#import <MetaZKit/MZBaseSearchProvider.h>

#import <MetaZKit/NSArray+Mapping.h>
#import <MetaZKit/NSDate+UTC.h>
Expand Down
8 changes: 8 additions & 0 deletions MetaZ.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
1B040EDF1081B53F0027FA0D /* tagChimp.png in Resources */ = {isa = PBXBuildFile; fileRef = 1B040EDE1081B53F0027FA0D /* tagChimp.png */; };
1B040EE21081B5550027FA0D /* TCSearchProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 1B040EE11081B5550027FA0D /* TCSearchProvider.m */; };
1B040EEE1081C0110027FA0D /* TagChimp.mzplugin in Copy Plugins */ = {isa = PBXBuildFile; fileRef = 1B040EBE1081B3C00027FA0D /* TagChimp.mzplugin */; };
1B0604AD1199E24700589ACE /* MZBaseSearchProvider.h in Headers */ = {isa = PBXBuildFile; fileRef = 1B0604AB1199E24700589ACE /* MZBaseSearchProvider.h */; settings = {ATTRIBUTES = (Public, ); }; };
1B0604AE1199E24700589ACE /* MZBaseSearchProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 1B0604AC1199E24700589ACE /* MZBaseSearchProvider.m */; };
1B060DF910E267810009D847 /* imdb_plugin.rb in Resources */ = {isa = PBXBuildFile; fileRef = 1B060DF810E267810009D847 /* imdb_plugin.rb */; };
1B060E5A10E26BA90009D847 /* IMDBSearch.m in Sources */ = {isa = PBXBuildFile; fileRef = 1B060E5910E26BA90009D847 /* IMDBSearch.m */; };
1B060E6710E26F970009D847 /* IMDBSearchProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 1B060E6610E26F970009D847 /* IMDBSearchProvider.m */; };
Expand Down Expand Up @@ -439,6 +441,8 @@
1B040EDE1081B53F0027FA0D /* tagChimp.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; name = tagChimp.png; path = Plugins/TagChimp/resources/tagChimp.png; sourceTree = "<group>"; };
1B040EE01081B5550027FA0D /* TCSearchProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TCSearchProvider.h; sourceTree = "<group>"; };
1B040EE11081B5550027FA0D /* TCSearchProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TCSearchProvider.m; sourceTree = "<group>"; };
1B0604AB1199E24700589ACE /* MZBaseSearchProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MZBaseSearchProvider.h; sourceTree = "<group>"; };
1B0604AC1199E24700589ACE /* MZBaseSearchProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MZBaseSearchProvider.m; sourceTree = "<group>"; };
1B060DF810E267810009D847 /* imdb_plugin.rb */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.ruby; path = imdb_plugin.rb; sourceTree = "<group>"; };
1B060E5810E26BA90009D847 /* IMDBSearch.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IMDBSearch.h; sourceTree = "<group>"; };
1B060E5910E26BA90009D847 /* IMDBSearch.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = IMDBSearch.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1073,6 +1077,8 @@
1B3AD7DB1105829500B33F85 /* MZOperationsController.m */,
1BAD35FA118D4C470052DE3B /* MZReadOperationsController.h */,
1BAD35FB118D4C470052DE3B /* MZReadOperationsController.m */,
1B0604AB1199E24700589ACE /* MZBaseSearchProvider.h */,
1B0604AC1199E24700589ACE /* MZBaseSearchProvider.m */,
1B0330AB10B1010700B28E3A /* NSDate+UTC.h */,
1B0330AC10B1010700B28E3A /* NSDate+UTC.m */,
1BAD35FC118D4C470052DE3B /* NSFileManager+MZCreate.h */,
Expand Down Expand Up @@ -1944,6 +1950,7 @@
1BAD3600118D4C470052DE3B /* NSFileManager+MZCreate.h in Headers */,
1BC256381192C183001B9E1E /* NSObject+WaitUntilChange.h in Headers */,
1BEC79071197B74400337403 /* GTMStackTrace.h in Headers */,
1B0604AD1199E24700589ACE /* MZBaseSearchProvider.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -2445,6 +2452,7 @@
1BAD3601118D4C470052DE3B /* NSFileManager+MZCreate.m in Sources */,
1BC256391192C183001B9E1E /* NSObject+WaitUntilChange.m in Sources */,
1BEC79061197B73E00337403 /* GTMStackTrace.m in Sources */,
1B0604AE1199E24700589ACE /* MZBaseSearchProvider.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
1 change: 1 addition & 0 deletions Plugins/Amazon/src/AmazonSearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
NSDictionary* ratingsMap;
}

+ (id)searchWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params;
- (id)initWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params;

@end
16 changes: 15 additions & 1 deletion Plugins/Amazon/src/AmazonSearch.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ + (Class)restWrapper
return [AmazonRequest class];
}

+ (id)searchWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params
{
return [[[self alloc] initWithProvider:provider delegate:delegate url:url parameters:params] autorelease];
}

- (id)initWithProvider:(id)theProvider delegate:(id<MZSearchProviderDelegate>)theDelegate url:(NSURL *)url parameters:(NSDictionary *)params;
{
self = [super initWithProvider:theProvider delegate:theDelegate url:url usingVerb:@"GET" parameters:params];
Expand Down Expand Up @@ -118,7 +123,6 @@ - (void)dealloc
{
[mapping release];
[ratingsMap release];
MZLoggerDebug(@"AmazonSearch release:\n%@", GTMStackTrace());
[super dealloc];
}

Expand Down Expand Up @@ -202,4 +206,14 @@ - (void)wrapper:(MZRESTWrapper *)theWrapper didRetrieveData:(NSData *)data
[super wrapper:theWrapper didRetrieveData:data];
}

- (void)addObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context
{
[super addObserver:observer forKeyPath:keyPath options:options context:context];
}

- (void)removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath
{
[super removeObserver:observer forKeyPath:keyPath];
}

@end
3 changes: 1 addition & 2 deletions Plugins/Amazon/src/AmazonSearchProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
#import <MetaZKit/MetaZKit.h>
#import "AmazonSearch.h"

@interface AmazonSearchProvider : NSObject <MZSearchProvider>
@interface AmazonSearchProvider : MZBaseSearchProvider <MZSearchProvider>
{
AmazonSearch* search;
NSImage* icon;
NSArray* supportedSearchTags;
NSMenu* menu;
Expand Down
15 changes: 4 additions & 11 deletions Plugins/Amazon/src/AmazonSearchProvider.m
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ @implementation AmazonSearchProvider

- (void)dealloc
{
[search release];
[icon release];
[supportedSearchTags release];
[menu release];
Expand Down Expand Up @@ -110,22 +109,16 @@ - (BOOL)searchWithData:(NSDictionary *)data
}
}

if(search)
{
// Finish last search;
[search cancel];
[search waitUntilFinished];
[search release];
search = nil;
}

[self cancelSearch];

if(!reallyDoSearch)
return NO;

MZLoggerDebug(@"Sent request to Amazon:");
for(NSString* key in [params allKeys])
MZLoggerDebug(@" '%@' -> '%@'", key, [params objectForKey:key]);
search = [[AmazonSearch alloc] initWithProvider:self delegate:delegate url:searchURL parameters:params];
AmazonSearch* search = [AmazonSearch searchWithProvider:self delegate:delegate url:searchURL parameters:params];
[self startSearch:search];
[queue addOperation:search];
return YES;
}
Expand Down
1 change: 1 addition & 0 deletions Plugins/TagChimp/src/TCSearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
NSDictionary* mapping;
}

+ (id)searchWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params;
- (id)initWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params;

@end
7 changes: 6 additions & 1 deletion Plugins/TagChimp/src/TCSearch.m
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@

@implementation TCSearch

- (id)initWithProvider:(id)theProvider delegate:(id<MZSearchProviderDelegate>)theDelegate url:(NSURL *)url parameters:(NSDictionary *)params;
+ (id)searchWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate url:(NSURL *)url parameters:(NSDictionary *)params
{
return [[[self alloc] initWithProvider:provider delegate:delegate url:url parameters:params] autorelease];
}

- (id)initWithProvider:(id)theProvider delegate:(id<MZSearchProviderDelegate>)theDelegate url:(NSURL *)url parameters:(NSDictionary *)params
{
self = [super initWithProvider:theProvider delegate:theDelegate url:url usingVerb:@"GET" parameters:params];
if(self)
Expand Down
3 changes: 1 addition & 2 deletions Plugins/TagChimp/src/TCSearchProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
#import <MetaZKit/MetaZKit.h>
#import "TCSearch.h"

@interface TCSearchProvider : NSObject <MZSearchProvider>
@interface TCSearchProvider : MZBaseSearchProvider <MZSearchProvider>
{
TCSearch* search;
NSImage* icon;
NSArray* supportedSearchTags;
NSMenu* menu;
Expand Down
14 changes: 4 additions & 10 deletions Plugins/TagChimp/src/TCSearchProvider.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ @implementation TCSearchProvider

- (void)dealloc
{
[search release];
[icon release];
[supportedSearchTags release];
[menu release];
Expand Down Expand Up @@ -144,21 +143,16 @@ - (BOOL)searchWithData:(NSDictionary *)data

[params setObject:@"200" forKey:@"limit"];

if(search)
{
// Finish last search;
[search cancel];
[search waitUntilFinished];
[search release];
search = nil;
}
[self cancelSearch];

if([title length] == 0 && !supportsEmptyTitle)
return NO;

MZLoggerDebug(@"Sent request to tagChimp:");
for(NSString* key in [params allKeys])
MZLoggerDebug(@" '%@' -> '%@'", key, [params objectForKey:key]);
search = [[TCSearch alloc] initWithProvider:self delegate:delegate url:searchURL parameters:params];
TCSearch* search = [TCSearch searchWithProvider:self delegate:delegate url:searchURL parameters:params];
[self startSearch:search];
[queue addOperation:search];
return YES;
}
Expand Down
1 change: 1 addition & 0 deletions Plugins/TheTVDB/src/TheTVDBSearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
@property(readonly) id provider;
@property(readonly) id<MZSearchProviderDelegate> delegate;

+ (id)searchWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate queue:(NSOperationQueue *)queue;
- (id)initWithProvider:(id)provider delegate:(id<MZSearchProviderDelegate>)delegate queue:(NSOperationQueue *)queue;

- (void)queueOperation:(NSOperation *)operation;
Expand Down
Loading

0 comments on commit 6573a8c

Please sign in to comment.