It looks like your current system has three fundamental events:
- Operation is added to the queue
- Operation sends notification while executing
- Operation completion block is called
Unless the queue itself explicitly listens for any NSNotifications
that might be sent by the blocks, it has no way of knowing whether they have happened yet. But even if it does listen, the ordering in which observers of NSNotifications
are called is non-deterministic. In other words, even if the queue listens for the notification and interlocks its callback with enqueue/dequeue operations, it could (and eventually would) still be too late for another client to start listening for that NSNotification
, and you would falsely reject an operation.
Consider this alternative: Instead of using the completion block to manage the identifier list, use the notification itself -- have the queue handle sending the notifications. Put differently, let's get rid of the third event and have the notification sending do double duty for identifier list maintenance. The simplest way I came up with to do this looked like:
Header:
//
// SONotifyingOperationQueue.h
// NotifyingOpQueue
//
typedef void (^SOSendNotificationBlock)(NSDictionary* userInfo);
typedef void (^SONotifyingBlock)(SOSendNotificationBlock sendNotificationBlock);
@interface SONotifyingOperationQueue : NSOperationQueue
- (BOOL)addOperationForBlock:(SONotifyingBlock)block withNotificationName:(NSString*)notificationName;
@end
Implementation
//
// SONotifyingOperationQueue.m
// NotifyingOpQueue
//
#import "SONotifyingOperationQueue.h"
@implementation SONotifyingOperationQueue
{
NSMutableSet* _names;
}
- (BOOL)addOperationForBlock: (SONotifyingBlock)block withNotificationName: (NSString*)notificationName
{
notificationName = [[notificationName copy] autorelease];
BOOL shouldAdd = NO;
@synchronized(self)
{
_names = _names ? : [[NSMutableSet alloc] init];
if (![_names containsObject: notificationName])
{
[_names addObject: notificationName];
shouldAdd = YES;
}
}
if (shouldAdd)
{
NSBlockOperation* blockOp = [[[NSBlockOperation alloc] init] autorelease];
__block SONotifyingOperationQueue* blockSelf = self;
SOSendNotificationBlock notificationBlock = ^(NSDictionary* userInfo){
@synchronized(blockSelf)
{
[blockSelf->_names removeObject: notificationName];
// Sending the notification from inside the @synchronized makes it atomic
// with respect to enqueue operations, meaning there can never be a missed
// notification that could have been received.
[[NSNotificationCenter defaultCenter] postNotificationName: notificationName object: blockSelf userInfo: userInfo];
}
};
dispatch_block_t executionBlock = ^{
block(notificationBlock);
};
[blockOp addExecutionBlock: executionBlock];
[self addOperation: blockOp];
}
return shouldAdd;
}
- (void)dealloc
{
[_names release];
[super dealloc];
}
@end
This approach makes several changes to your original approach. First, the API here adds blocks and not NSOperations
. You could do the same thing with an NSOperation
subclass, but it would be more code, and wouldn't change the overall pattern. It also merges the notion of the identifier and the notification name. If an operation could send multiple, different NSNotifications
, this won't work without modification, but again, the overall pattern would be the same. The important feature of this pattern is that your id/name check is now interlocked with the notification sending itself, providing a strong guarantee that if someone goes to add a new block/operation to the queue, and another operation with the same id/name hasn't fired its notification yet, the new operation won't be added, but if the notification has been fired, then it will be added, even if the preceding block hasn't yet completed.
If having the NSOperation
object was somehow important here, you could also have the method here return the operation it creates for the supplied block.
HTH.