Passing blocks to asynchronous methods

Question

I'm passing a block to an asynchronous method which executes this block later. My app crashes if I don't copy the block before passing it to someMethod:success:failure:

Is there a way to copy the blocks in forwardInvocation: instead of copying it before passing it to someMethod:success:failure: ?

The flow is someMethod:success:failure: -> forwardInvocation: -> httpGet:success:failure

httpGet:success:failure: executes the success or the failure block depending on the HTTP status code.

// AppDelegate.h

@interface AppDelegate : UIResponder <UIApplicationDelegate>

@property (strong, nonatomic) id response;
@property (strong, nonatomic) NSError *error;

@end

// AppDelegate.m

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
    // The app crashes if the blocks are not copied here!
    [[MyController new] someMethod:[^(NSString *response) {
        self.response = response;
        NSLog(@"response = %@", response);
    } copy] failure:[^(NSError *error) {
        self.error = error;
    } copy]];

    return YES;
}

@end

// MyController.h

@protocol MyControllerProtocol <NSObject>

@optional


- (void)someMethod:(void (^)(NSString *response))success
           failure:(void (^)(NSError *error))failure;

@end

@interface MyController : NSObject <MyControllerProtocol>

@end

// MyController.m

#import "MyController.h"
#import "HTTPClient.h"

@implementation MyController

- (void)forwardInvocation:(NSInvocation *)invocation {
    [invocation retainArguments];

    NSUInteger numberOfArguments = [[invocation methodSignature] numberOfArguments];

    typedef void(^SuccessBlock)(id object);
    typedef void(^FailureBlock)(NSError *error);

    __unsafe_unretained SuccessBlock successBlock1;
    __unsafe_unretained SuccessBlock failureBlock1;
    [invocation getArgument:&successBlock1 atIndex:(numberOfArguments - 2)]; // success block is always the second to last argument (penultimate)
    SuccessBlock successBlock = [successBlock1 copy];
    [invocation getArgument:&failureBlock1 atIndex:(numberOfArguments - 1)]; // failure block is always the last argument
    FailureBlock failureBlock = [failureBlock1 copy];

    NSLog(@"successBlock copy = %@", successBlock);
    NSLog(@"failureBlock copy = %@", failureBlock);

    // Simulates a HTTP request and calls the success block later!
    [HTTPClient httpGet:@"somerequest" success:successBlock failure:failureBlock];
}

- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel {
    NSMethodSignature *methodSignature = [super methodSignatureForSelector:sel];
    return methodSignature;
}

@end


// HTTPClient.h

@interface HTTPClient : NSObject

+ (void)httpGet:(NSString *)path
        success:(void (^)(id object))success
        failure:(void (^)(NSError *error))failure;

@end

// HTTPClient.m

#import "HTTPClient.h"

@implementation HTTPClient

+ (void)httpGet:(NSString *)path
        success:(void (^)(id object))success
        failure:(void (^)(NSError *error))failure
{
    // Invoke the method.
    double delayInSeconds = 2.0;
    dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delayInSeconds * NSEC_PER_SEC));
    dispatch_after(popTime, dispatch_get_main_queue(),
               ^{
                   success(@"foo");
               });
}

@end

The complete source code can be found here: https://github.com/priteshshah1983/BlocksWithNSInvocation

Can you please help?

Minestrone

We would need to see someMethod:failure: I don't see that anywhere in your code. – Munger

@Munger someMethod:success:failure: is not implemented. Hence, methodSignatureForSelector: and then forwardInvocation: would be called. – Minestrone

@Munger I fixed some typos, sorry! It is supposed to be someMethod:success:failure: – Minestrone

Ah sorry, I misunderstood. I would guess that copy here works only because it happens to be creating a strong reference to the block where you otherwise don't have one. Declaring and initializing them as __strong before passing them would probably have the same affect. – Munger

It turns out the lack of strong reference was the issue. Doesn't passing in as a parameter create a strong reference automatically? Are blocks special in that sense? – Minestrone

@Munger Please post your comment as an answer so that I can mark it as the accepted answer. – Minestrone

Passing a parameter has no effect on the persistence of the object. If you look at your code you'll find anything you pass is retained by you or something else. In many cases it happens in a convenience constructor such as stringWithFormat: which returns an autoreleased object. – Munger

@pshah: The reason that assigning it to a variable first seemed to get rid of the crash in your case, is because your version of the ARC compiler happened to insert a call to copy for the block in the case where it is assigned to a variable, but not in the case when it's used directly. Modern versions of the ARC compiler have become very liberal about inserting copies everywhere for block types, since it's never incorrect to copy a block. However, early versions didn't do this, and nowhere in the ARC specification does it guarantee this, so it's not guaranteed to work. – Mott

Answer 1

The culprit is the line [invocation retainArguments]. If you comment out that line, it works fine. (That line was never necessary anyway because the invocation is never stored or used asynchronously.)

Explanation:

Think about what -retainArguments does. It calls retain on all the arguments of object pointer type. And then when the invocation is deallocated, it calls release on them.

But the arguments are stack (non-copied) blocks. retain and release have no effect on it (since it's not a heap object). So when it is retained, nothing happens, and then (from the crash) it appears that the invocation was autoreleased at some point (a perfectly normal thing to happen), so the final release and deallocation of the invocation happens asynchronously. When the invocation is deallocated, it tries to release its retained arguments, but by then it's trying to message a no-longer valid stack block, causing a crash.

P.S. the copying of the blocks in forwardInvocation: was also unnecessary

Mott