NSOperation remains in an NSOperationQueue after cancellation

Question

I perform downloading images using NSOperation and an NSOperationQueue. Every Operation retains a StoreThumbRequest object encapsulating all request specific data including a target view, waiting for an image. At the same time this view retains an NSOperation object to cancel an operation during reusing or deallocations. This retain loop is carefully broken at 'cancel' and 'main' methods in appropriate times.

I found an NSOperation to remain in a NSOperationQueue when set a limit on its maxConcurrentOperationsCount. Reaching this limit prevented 'main' method of new NSOperations from being called. NSOperation remains in a queue only when it's cancelled. If it managed to finish its task, it is gracefully removed from its NSOperationQueue.

My NSOperations are non-concurrent and have no dependencies.

Any suggestions? Thanx in advance

#import "StoreThumbLoadOperation.h"
#import "StoreThumbCache.h"

@interface StoreThumbLoadOperation () <NSURLConnectionDelegate> {
    StoreThumbRequest *_request;
    NSMutableData *_downloadedData;
    NSURLConnection *_connection;
    NSPort *_port;
}

@end

@implementation StoreThumbLoadOperation

-(id)initWithRequest: (StoreThumbRequest *)request
{
    NSParameterAssert(request);
    self = [super init];
    if (self) {
        _request = request;
    }
    return self;
}

-(void)main
{
    NSURL *url = ...;

    NSURLRequest *request = [NSURLRequest requestWithURL: url];
    _connection = [[NSURLConnection alloc] initWithRequest: request delegate: self startImmediately: NO];

    NSRunLoop *runLoop = [NSRunLoop currentRunLoop];
    _port = [NSMachPort port];
    [runLoop addPort: _port forMode: NSDefaultRunLoopMode];
    [_connection scheduleInRunLoop: runLoop forMode: NSDefaultRunLoopMode];
    [_connection start];
    [runLoop run];
}

-(void)cancel
{
    [super cancel];

    [_connection unscheduleFromRunLoop: [NSRunLoop currentRunLoop] forMode: NSDefaultRunLoopMode];
    [_connection cancel];
    _request.thumbView.operation = nil; //break retain loop
    [[NSRunLoop currentRunLoop] removePort: _port forMode: NSDefaultRunLoopMode];
}

#pragma mark - NSURLConnectionDelegate

- (void)connection:(NSURLConnection *)connection didReceiveResponse:(NSURLResponse *)response
{
    _downloadedData = [NSMutableData new];
}

- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data
{
    [_downloadedData appendData: data];
}

- (void)connectionDidFinishLoading:(NSURLConnection *)connection
{
    if (_connection == connection) {
        NSFileManager *fileManager = [NSFileManager new];
        NSString *pathForDownloadedThumb = [[StoreThumbCache sharedCache] thumbPathOnRequest: _request];
        NSString *pathForContainigDirectory = [pathForDownloadedThumb stringByDeletingLastPathComponent];
        if (! [fileManager fileExistsAtPath: pathForContainigDirectory]) {
            //create a directory if required
            [fileManager createDirectoryAtPath: pathForContainigDirectory withIntermediateDirectories: YES attributes: nil error: nil];
        }

        if (! self.isCancelled) {
            [_downloadedData writeToFile: pathForDownloadedThumb atomically: YES];
            UIImage *image = [UIImage imageWithContentsOfFile: pathForDownloadedThumb];

            //an image may be empty
            if (image) {
                if (! self.isCancelled) {
                    [[StoreThumbCache sharedCache] setThumbImage: image forKey: _request.cacheKey];

                    if (_request.targetTag == _request.thumbView.tag) {
                        dispatch_async(dispatch_get_main_queue(), ^{
                            _request.thumbView.image = image;
                        });
                    }
                }
            }
             _request.thumbView.operation = nil; //break retain loop
            [[NSRunLoop currentRunLoop] removePort: _port forMode: NSDefaultRunLoopMode];
        }
    }
}


- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
{
    if (error.code != NSURLErrorCancelled) {
        #ifdef DEBUG
        NSLog(@"failed downloading thumb for name: %@ with error %@", _request.thumbName, error.localizedDescription);
        #endif

        _request.thumbView.operation = nil;
        [[NSRunLoop currentRunLoop] removePort: _port forMode: NSDefaultRunLoopMode];
    }
}

@end

I understand, that an NSOperation is not expected to be immediately removed from its NSOperationQueue. But it remains there for an unlimited time period

From NSOperation cancel method documentation

This method does not force your operation code to stop. Instead, it updates the object’s internal flags to reflect the change in state.

Answer 1

I believe this is happening because the runloop (which you didn't remove when you intended to end the operation) keeps the operation alive.

Replace all instances of the following line :

[[NSRunLoop currentRunLoop] removePort: _port forMode: NSDefaultRunLoopMode];

with this one :

[[NSRunLoop currentRunLoop] removePort: _port forMode: NSDefaultRunLoopMode];
CFRunLoopStop(CFRunLoopGetCurrent());

Answer 2

It turned out, that in the 'main' method I did not check if the operation was already cancelled. I also did not take special considerations about 'cancel' method being called from the main thread

I also did not check, whether the 'runLoop' ever stopped. It usually stops if no input sources attached, so I was removing the port from the loop. I now changed the way I run the NSRunLoop using 'runMode:beforeDate:'.

Here is the updated working code

#define STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED() \
if (self.cancelled) {\
    [self internalComplete]; \
    return; \
}

@interface StoreThumbLoadOperation () <NSURLConnectionDelegate>
@property (strong, nonatomic) StoreThumbRequest *request;
@property (strong, nonatomic) NSMutableData *downloadedData;
@property (strong, nonatomic) NSURLConnection *connection;
@property (strong, nonatomic) NSPort *port;
@property (strong, nonatomic) NSRunLoop *runLoop;
@property (strong, nonatomic) NSThread *thread;
@property (assign, nonatomic) unsigned long long existingOnDiskThumbWeightInBytes;
@property (assign, nonatomic) BOOL isCompleted;
@property (assign, nonatomic) BOOL needsStop;
@end

@implementation StoreThumbLoadOperation

-(id)initWithRequest: (StoreThumbRequest *)request existingCachedImageSize:(unsigned long long)bytes
{
    NSParameterAssert(request);
    self = [super init];
    if (self) {
        self.request = request;
        self.existingOnDiskThumbWeightInBytes = bytes;
    }
    return self;
}

-(void)main
{
    // do not call super for optimizations
    //[super main];

    STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED();

    @autoreleasepool {
        NSURL *url = ...;

        NSURLRequest *request = [NSURLRequest requestWithCredentialsFromUrl:url];
        self.connection = [[NSURLConnection alloc] initWithRequest: request delegate: self startImmediately: NO];

        self.runLoop = [NSRunLoop currentRunLoop];
        self.port = [NSMachPort port];
        self.thread = [NSThread currentThread];  // <- retain the thread

        [self.runLoop addPort: self.port forMode: NSDefaultRunLoopMode];
        [self.connection scheduleInRunLoop: self.runLoop forMode: NSDefaultRunLoopMode];

        [self.connection start];

        // will run the loop until the operation is not cancelled or the image is not downloaded
        NSTimeInterval stepLength = 0.1;
        NSDate *future = [NSDate dateWithTimeIntervalSinceNow:stepLength];
        while (! self.needsStop && [self.runLoop runMode:NSDefaultRunLoopMode beforeDate:future]) {
            future = [future dateByAddingTimeInterval:stepLength];
        }

        // operation is cancelled, or the image is downloaded
        self.isCompleted = YES;
        [self internalComplete];
    }
}

-(void)cancel {
    [super cancel];
    STORE_THUMB_LOAD_OPERATION_LOG_STATUS(@"cancelled");
    [self setNeedsStopOnPrivateThread];
}

- (BOOL)isFinished {
    // the operation must become completed to be removed from the queue
    return self.isCompleted || self.isCancelled;
}

#pragma mark - privates

- (void)setNeedsStopOnPrivateThread {
  // if self.thread is not nil, that the 'main' method was already called. if not, than the main thread cancelled the operation before it started
  if (! self.thread || [NSThread currentThread] == self.thread) {
     [self internalComplete];
  } else {
     [self performSelector:@selector(internalComplete) onThread:self.thread withObject:nil waitUntilDone:NO];
  }
}

- (void)internalComplete {
    [self cleanUp];
    self.needsStop = YES; // <-- will break the 'while' loop
}

- (void)cleanUp {
    [self.connection unscheduleFromRunLoop: self.runLoop forMode: NSDefaultRunLoopMode];
    [self.connection cancel];
    self.connection = nil;

    //break retain loop
    self.request.thumbView.operation = nil;

    [self.runLoop removePort: self.port forMode: NSDefaultRunLoopMode];
    self.port = nil;
    self.request = nil;
    self.downloadedData = nil;
}

#pragma mark - NSURLConnectionDelegate

- (void)connection:(NSURLConnection *)connection didReceiveResponse:(NSURLResponse *)response {
    self.downloadedData = [NSMutableData new];
}

- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data {
    [self.downloadedData appendData: data];
}

- (void)connectionDidFinishLoading:(NSURLConnection *)connection {
    if (self.connection != connection)
        return;

    STORE_THUMB_LOAD_OPERATION_LOG_STATUS(@"entered connection finished");

    //create a directory if required

    STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED();

    // wright the image to the file

    STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED();

    // create the image from the file
    UIImage *image = [UIImage imageWithContentsOfFile:...];

    STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED();

    if (image) {
        // cache the image

        STORE_THUMB_LOAD_OPERATION_RETURN_IF_CANCELLED();

        // update UI on the main thread
        if (self.request.targetTag == self.request.thumbView.tag) {
            StoreThumbView *targetView = self.request.thumbView;
            dispatch_async(dispatch_get_main_queue(), ^{
                targetView.image = image;
            });
        }
    }

    [self setNeedsStopOnPrivateThread];
}

- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error {
    if (error.code != NSURLErrorCancelled) {            
        [self cancel];
    }
}

@end