Search code examples
iosnsurlconnectionnsoperationnsoperationqueuensrunloop

NSOperation remains in an NSOperationQueue after cancellation


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.


Solution

  • 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