EXC_BAD_ACCESS when nesting [Promise new:...] calls

[Glavin001]

Result:

I am wrapping the internals of many asynchronous method's with

    return [PMKPromise new:^(PMKPromiseFulfiller fulfiller, PMKPromiseRejecter rejecter) {

and am now finding that EXC_BAD_ACCSS issue, as mentioned on the website:

2. EXC_BAD_ACCESS
ARC is pretty good, but in some cases it is possible for your Promise chain to be partially deallocated. Usually when wrapping delegate systems. PromiseKit itself has macros that force additional retains and releases to avoid this.

I have been "debugging" this for hours, however I can find little helpful information or correlation except that it stops if I flatten the Promise chain a little and then occurs later on.

I am hoping that you may know what I am talking about / seen this before / have a solution. I have been up all night working on this and implemented PromiseKit everywhere and not am a little discouraged to see I have nothing to show for my nights work because of a strange error that I cannot debug.

Please let me know if there is anything I can do to help you diagnose. Thank you!

[mxcl]

Can you show the code?

Nesting news is fine, PromiseKit itself does this all over the place.

Usually something is being deallocated because you need to keep a strong pointer to that thing around, and if I could see the code I would be able to advise.

[Glavin001]

Here is the project I am working on, specifically this file: https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m

[mxcl]

Thanks, I'll check it out.

[mxcl]

As an FYI, this:

- (PMKPromise *) find:(Class)modelClass withQuery:(NSDictionary *)query;
{
    return [PMKPromise new:^(PMKPromiseFulfiller fulfiller, PMKPromiseRejecter rejecter) {
        [self.adapter findQuery:modelClass withQuery:query withStore:self]
        .then(^(NSDictionary *adapterPayload) {
            dispatch_async(dispatch_get_global_queue( DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void){
                //Background Thread
                // FIXME: Should used a shared Serializer, etc.
                SLSerializer *serializer = [[SLSerializer alloc] init];
                // Extract from Payload
                NSArray *extractedPayload = [serializer extractArray:modelClass withPayload:adapterPayload withStore:self];
                dispatch_async(dispatch_get_main_queue(), ^(void){
                    //Run UI Updates
                    [self pushMany:modelClass withData:extractedPayload]
                    .then(^(NSArray *records) {
                        fulfiller(records);
                    })
                    .catch(rejecter);
                });
            });
        })
        .catch(rejecter);
    }];
}

Could be simpler:

- (PMKPromise *) find:(Class)modelClass withQuery:(NSDictionary *)query;
{
    id q = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);

    return [self.adapter findQuery:modelClass withQuery:query withStore:self].thenOn(q, ^(NSDictionary *adapterPayload) {
        SLSerializer *serializer = [[SLSerializer alloc] init];
        NSArray *extractedPayload = [serializer extractArray:modelClass withPayload:adapterPayload withStore:self];
        return [self pushMany:modelClass withData:extractedPayload];
    });
}

You don't have to do any GCD with promises, just specify what queue you want in the continuation handlers.

[Glavin001]

Even more specifically, around this: https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m#L226-L252
which calls https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m#L67-L94 and https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m#L278-L348

I hope that helps! Looking at my commits, I had about a lull of 4 hours last night/this morning where I was receiving this error and could not figure it out. And something as strange as commenting out the contents of the then block would allow it to not error, however, of course the program would not pass the tests because it was missing steps.

[mxcl]

Also there's an AFNetworking+PromiseKit pod now: https://github.com/csotiriou/AFNetworking-PromiseKit in case that is useful.

[Glavin001]

Oh wow, that's awesome!! Thank you for that optimization, @mxcl! I am still learning all of the exciting PromiseKit tricks and you've been a great help so far.

[mxcl]

This:

- (PMKPromise *) push:(Class)modelClass withData:(NSDictionary *)datum
{
    return [PMKPromise new:^(PMKPromiseFulfiller fulfiller, PMKPromiseRejecter rejecter) {

        NSLog(@"push: %@",datum);
        SLNid nid = (NSString *) datum[@"nid"];

        [PMKPromise when:@[
                           [self record:modelClass forId:nid],
                           [self normalizeRelationships:modelClass withData:datum withStore:self]
                           ]
         ]
        .then(^(NSArray *results) {
            SLModel *record = results[0];
            NSDictionary *newDatum = results[1];
            NSLog(@"record: %@", record);
            NSLog(@"post normalizeRelationships datum: %@", newDatum);

            //
            [record setupData:newDatum];
            NSLog(@"Pushed record: %@", record);

            fulfiller(record);

        })
        .catch(rejecter);
    }];
}

Could be:

- (PMKPromise *) push:(Class)modelClass withData:(NSDictionary *)datum
{
    NSLog(@"push: %@",datum);
    SLNid nid = (NSString *) datum[@"nid"];

    return [PMKPromise when:@[
                       [self record:modelClass forId:nid],
                       [self normalizeRelationships:modelClass withData:datum withStore:self]
                       ]
     ]
    .then(^(NSArray *results) {
        SLModel *record = results[0];
        NSDictionary *newDatum = results[1];
        NSLog(@"record: %@", record);
        NSLog(@"post normalizeRelationships datum: %@", newDatum);

        //
        [record setupData:newDatum];
        NSLog(@"Pushed record: %@", record);

        return record; 
    });
}

There's no reason to nest things that return promises in a +new:.

Obviously it shouldn’t crash either way.

[Glavin001]

I had been trying to go through and clean up when I read that if you return a Promise it will resolve it, so I could omit the [PMKPromise new..] part. That's really great how condensed it really can go!

[mxcl]

A pleasure to help as it enables me to reconsider the documentation so people are guided towards the more condensed versions in future.

[mxcl]

I have trouble running the tests so I can reproduce, is there some step I missed?

[Glavin001]

I think I may have committed a typo in my morning rush to my office. I'll let you know once I've fixed that error. Working only now.

[Glavin001]

Hey @mxcl. I apologize for not getting back to you sooner: I ended up having other high priorities to do at work and was unavailable for the weekend.

Would you be able to try running the tests again and let me know how it goes and potentially what it shows in the console logs? I will be pushing to master branch with what I have been working on and appears to work for me, so you can successfully run tests such as testFindAllAttributes and others.

[mxcl]

k np,, I'll look into it later today.

[Glavin001]

I have stepped through the code and found for case where it breaks the sig is returned as nil. So when the following line below is executed it results in EXC_BAD_ACCESS (code=2 address=0x0):

// sig = nil
const char rtype = sig.methodReturnType[0];

See https://github.com/mxcl/PromiseKit/blob/master/objc/PMKPromise.m#L42-L44

Important

I may have forgot to mention: I am seeing the error on the front-end application for iOS I am developing, that uses the Streamlyne Cocoa SDK that is public. However, the iOS app I am developing is closed source, so I am unable to show you the complete source code.
I am hoping that you recognize why the sig would be nil and I could look into it further on your behalf and get back to you with any relevant information that would help debugging my usage.

[Glavin001]

On my iOS app I have debugged it down to the following snippet of code:

    NSLog(@"Attempt login");
    [client authenticateWithUserEmail:email
                              withPassword:password
                          withOrganization:organization]
    .then(^(SLClient *client, SLUser *me) {
        NSLog(@"Logged in");
        // Removed for brevity
    }).catch(^(NSError *error) {
     // Removed for brevity
   });

It successfully logs Attempt login and crashes before Logged in. See authenticateWithUserEmail:withPassword:withOrganization here.

Here is a sample of my console log (I redacted the unimportant information, just wanted to show you the starting message of those logs so you know where it is flowing):

2014-08-25 13:06:09:112 Streamlyne[58980:60b] Attempt login
2014-08-25 13:06:09.113 Streamlyne[58980:60b] absPath: /api/v1/me
2014-08-25 13:06:09.113 Streamlyne[58980:60b] Full path: http://54.183.15.175:5000/api/v1/me
2014-08-25 13:06:09.113 Streamlyne[58980:60b] Secret: REDACTED
2014-08-25 13:06:09.114 Streamlyne[58980:60b] theParams is empty.
2014-08-25 13:06:09.114 Streamlyne[58980:60b] HMAC message: GET:/api/v1/me:1408982829:
2014-08-25 13:06:09.114 Streamlyne[58980:60b] HMAC: REDACTED
2014-08-25 13:06:09.115 Streamlyne[58980:60b] GET http://54.183.15.175:5000/api/v1/me
2014-08-25 13:06:09.333 Streamlyne[58980:60b] Success, JSON: {
    // REDACTED
}
2014-08-25 13:06:15.710 Streamlyne[58980:60b] normalizeRelationships Payload: {
    // REDACTED
}
2014-08-25 13:06:16.235 Streamlyne[58980:60b] push: {
    // REDACTED
}
2014-08-25 13:06:16.251 Streamlyne[58980:1303] record:forId:, before find node
2014-08-25 13:06:16.251 Streamlyne[58980:60b] normalizeRelationships data: {
    // REDACTED
}
(lldb) 

• record:forId:, before find node is from https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m#L67-L95
• normalizeRelationships data is from https://github.com/Streamlyne/Cocoa-SDK/blob/master/Common/SLStore.m#L274-L344

I hope that helps! I believe I have used PromiseKit inappropriately in the Streamlyne Cocoa SDK and hopefully it is a simple fix / typo.

[Glavin001]

I believe I have found the issue! I am calling [PMKPromise when:arrayOfPromises], and in certain cases the array is initialled but empty (length of 0).
Looking deeper into the PromiseKit source code I found that you have already covered this case in + (PMKPromise *)all:(id<NSFastEnumeration, NSObject>)promises: https://github.com/mxcl/PromiseKit/blob/master/objc/PMKPromise%2BWhen.m#L24-L25

    if (count == 0)
        return [PMKPromise promiseWithValue:@[]];

That leads me to https://github.com/mxcl/PromiseKit/blob/master/objc/PMKPromise.m#L324-L329

+ (PMKPromise *)promiseWithValue:(id)value {
    PMKPromise *p = [PMKPromise alloc];
    p->_promiseQueue = PMKCreatePromiseQueue();
    p->_result = value ?: PMKNull;
    return p;
}

And that is as far as I have gotten so far to finding a solution. Manually adding checks in my SDK code to prevent from using PMKPromise when: with an empty array has resolved those cases thus far, however I feel that PromiseKit should handle this empty array of promises case, and I believe you are in agreement, since you've put the check in for length of 0.

I am going to try and look into how the promiseWithValue: works, however I may need help at this point. Hope what I've found is helpful! I think we are getting close to a resolution.

[mxcl]

Thanks for the assistance! I will really look at this soon. I've had a bunch of urgent client work that cannot wait sadly.

[Glavin001]

Not a problem. Let me know when you're working on it so I can help out :).

[dbachrach]

See #36 for the addition to all: to support empty promises array.

Relevant checkin: cbcbd9b

[Glavin001]

Thanks @dbachrach! I did find that commit and even the passing test; I was writing a comment here about my findings, but must have forgot to hit send.
So looks like there is still another issue at play.. 😦

[mxcl]

I still cannot reproduce with your project:

Test Case '-[Streamlyne_iOS_SDKTests testPushAssetWithRelationships]' started.
2014-08-29 10:18:51.758 xctest[15678:3231337] setUp
2014-08-29 10:18:51.760 xctest[15678:3231362] *** Assertion failure in +[NSManagedObjectContext MR_defaultContext], /Users/mxcl/scratch/Cocoa-SDK/Pods/MagicalRecord/MagicalRecord/Categories/NSManagedObjectContext/NSManagedObjectContext+MagicalRecord.m:60
2014-08-29 10:18:51.761 xctest[15678:3231362] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Default Context is nil! Did you forget to initialize the Core Data Stack?'

Is there some setup step I have missed?

[Glavin001]

Shoot. Sorry about that. Looks like I could be accessing a Core Data context from the wrong thread. I went through the other day and optimized most of the methods with the PromiseKit optimizations you suggested. I may have wrote another typo.. I'll take another look. Thanks

[Glavin001]

If I run all of the tests together, it errors like you have; however if I run them individually it appears to work (sometimes intermittently).

I will have to take a closer look later. Must be some form of side-effect between the tests..

[mxcl]

How am I meant to reproduce this issue? I thought running the test cases would do it, but you just showed that they all pass.

[Glavin001]

I am not entirely sure. The tests pass for the SDK, however when used in the closed source iOS application that uses this SDK they fail with something as simple as authenticateWithUserEmail:withPassword:withOrganization.
I was hoping, after reading the comment

1. EXC_BAD_ACCESS
   ARC is pretty good, but in some cases it is possible for your Promise chain to be partially deallocated. Usually when wrapping delegate systems. PromiseKit itself has macros that force additional retains and releases to avoid this.

that there may be something I should wrap my usage in to prevent from deallocating and causing the EXC_BAD_ACCESS errors.

I was also wondering if it could be an issue between the Mac and iOS version of PromiseKit. Since I am using the Mac version for the SDK tests and the iOS for the breaking app.

I have narrowed it down to a basic call to authenticateWithUserEmail:withPassword:withOrganization:organization,
see #77 (comment)

I'm not sure how else I can help. I wish there was some way I could distribute the reproducing App to you so you can experience what I am, however it's company IP.

I think I will try to build a basic iOS app as an open source project, just to demo a reproduction of the error I am experiencing. I hope that will help.

[Glavin001]

@mxcl here is a demo app that successfully reproduces the same error I am having: https://github.com/Streamlyne/cocoa-sdk-iso-demo

Let me know if there is anything else I can do! Thank you! :)

[mxcl]

K, if I force an anti-ARC retain on the block the crash doesn't happen. So I'm actually thinking we are exposing some bug in ARC here.

[mxcl]

Seems relevant: http://stackoverflow.com/questions/12061756/blocks-and-arc-copy-or-crash-with-release-build-caused-by-optimization-level

[mxcl]

0.9.16.3 pushed to CocoaPods.

[Glavin001]

It worked! Thank you!

[mxcl]

np. I should report this bug to Apple really. I'm pretty sure ARC is prematurely releasing the block based on my reading the spec.