Completion Handler causing EXC_BAD_ACCESS when same method is called twice

Question

I am working on some IAPs using this tutorial.

Firstly I fetch the products with this:

-(void)fetchAvailableProductsFirstLoad:(BOOL)firstTimeLoading {
    [[IAPHelper sharedInstance] requestProductsWithCompletionHandler:^(BOOL success, NSArray *products) { ...

The helper runs the following:

- (void)requestProductsWithCompletionHandler:(RequestProductsCompletionHandler)completionHandler {

    @synchronized(self) {
        // 1
        _completionHandler = [completionHandler copy];

        // 2
        _productsRequest = [[SKProductsRequest alloc] initWithProductIdentifiers:_productIdentifiers];
        _productsRequest.delegate = self;
        [_productsRequest start];
    }
}

When the products are returned or failed the following is called:

#pragma mark - SKProductsRequestDelegate

- (void)productsRequest:(SKProductsRequest *)request didReceiveResponse:(SKProductsResponse *)response {

    NSLog(@"Loaded list of products...");
    _productsRequest = nil;

    NSArray * skProducts = response.products;
    for (SKProduct * skProduct in skProducts) {
        NSLog(@"Found product: %@ %@ %0.2f",
              skProduct.productIdentifier,
              skProduct.localizedTitle,
              skProduct.price.floatValue);
    }

    _completionHandler(YES, skProducts);
    _completionHandler = nil;

}

- (void)request:(SKRequest *)request didFailWithError:(NSError *)error {

    NSLog(@"Failed to load list of products.");
    NSLog(@"Error: %@",error);
    _productsRequest = nil;

    _completionHandler(NO, nil);
    _completionHandler = nil;

}

Issue
The issue we have is when the user starts a fetch or products twice. For example the fetch products is called on the viewDidLoad, but if the user has a bad/slow connection and navigates away and then back to the controller. The initial fetch is not cancelled therefore there are two running.

I believe the issue is when the second is returned and the pointer has changed/does not exist/corrupt.

The EXC_BAD_ACCESS code 2 error occurs on the relevant line:

_completionHandler(YES, skProducts);

OR

_completionHandler(NO, nil);

Soundless

Answer 1

You're right. It does not exist when the second response is returned because it's nilled after the first response is handled: completionHandler = nil.

In this kind of situation, I find it safest to always check that the block exists before calling it:

if (_completionHandler) {
    _completionHandler(YES, skProducts);
    _completionHandler = nil;
}

(and the same in -request:didFailWithError:). In your current implementation, calling [[IAPHelper sharedInstance] requestProductsWithCompletionHandler:nil] would cause the same crash without this check (try it!).

On top of these safety checks, it would be best to cancel your first request when appropriate, like when the user navigates and won't see the response anyway. Also, in -requestProductsWithCompletionHandler:, either cancelling an existing _productsRequest before creating the new one or checking for an existing _productsRequest to decide whether or not to create a new one, would be another useful layer of safety.

Rolfrolfe

Thanks, I used parts of both answers but this was closest. I also added an additional method -(void)cancelProductRequest { [_productsRequest cancel]; _productsRequest = nil; } which cancels the current request if they navigate away from that controller to save there ever being multiple requests. – Soundless

Great @StuartM, definitely best to cancel the request if you don't need it. Glad I could help! – Rolfrolfe