MailCore / mailcore2

MailCore 2 provide a simple and asynchronous API to work with e-mail protocols IMAP, POP and SMTP. The API has been redesigned from ground up.
Other
2.6k stars 625 forks source link

EXC Bad Access #61

Closed ocrickard closed 11 years ago

ocrickard commented 11 years ago

I'm getting an error at line 59 in MCIMAPCopyMessageOperation:

    ErrorCode error;
    session()->session()->copyMessages(folder(), mUids, mDestFolder, &mDestUids, &error);
    MC_SAFE_RETAIN(mDestUids); <---- TROUBLE
    setError(error);

Here's the MCLog + backtrace:

2013-05-29 11:55:45.286 [9112:4f07] MCIMAPSession.cc:543: ssl connect imap.gmail.com 993 2
2013-05-29 11:55:45.287 [9112:4f07] MCIMAPSession.cc:587: connect ok
2013-05-29 11:55:45.287 [9112:4f07] MCIMAPSession.cc:618: login
2013-05-29 11:55:46.147 [9112:4e0b] MCOperationQueue.cc:71: quitting 0 0x1d595280
2013-05-29 11:55:46.173 [9112:4f07] MCIMAPSession.cc:753: login ok
2013-05-29 11:55:46.173 [9112:4f07] MCIMAPSession.cc:812: select
2013-05-29 11:55:46.266 [9112:main] MCOperationQueue.cc:132: trying to quit 0x216bb870
2013-05-29 11:55:46.266 [9112:c807] MCOperationQueue.cc:71: quitting 1 0x0
2013-05-29 11:55:46.266 [9112:c807] MCOperationQueue.cc:73: stopping 0x216bb870
2013-05-29 11:55:46.266 [9112:c807] MCOperationQueue.cc:107: cleanup thread 0x216bb870
2013-05-29 11:55:46.737 [9112:4f07] MCIMAPSession.cc:816: select error : 0(lldb) bt
* thread #12: tid = 0x3103, 0x002bdd76 Mercurius`mailcore::IMAPCopyMessagesOperation::main(this=0x1d595280) + 134 at MCIMAPCopyMessagesOperation.cc:59, stop reason = EXC_BAD_ACCESS (code=1, address=0x6000000c)
    frame #0: 0x002bdd76 Mercurius`mailcore::IMAPCopyMessagesOperation::main(this=0x1d595280) + 134 at MCIMAPCopyMessagesOperation.cc:59
    frame #1: 0x002a6fd6 Mercurius`mailcore::OperationQueue::runOperations(this=0x1e8bc360) + 586 at MCOperationQueue.cc:79
    frame #2: 0x002a6d86 Mercurius`mailcore::OperationQueue::runOperationsOnThread(queue=0x1e8bc360) + 14 at MCOperationQueue.cc:47
    frame #3: 0x3b3910e0 libsystem_c.dylib`_pthread_start + 308
dinhvh commented 11 years ago

mDestUids should be initialized with NULL. Could you check if other operations have similar issues? And pull request ;) Thanks!

ocrickard commented 11 years ago

OK, after a couple hours of trying to figure out what's going on here, it would seem to me that the uidSetResult is being released before the method copyMessages returns? Should there be a return before the release goto label?

Something like:

    return;
    release:
    MC_SAFE_RELEASE(uidSetResult);
    ...

However, now I'm having problems after this function returns. Specifically, line 95 in MCOOperationQueue:

performMethodOnMainThread((Object::Method) &OperationQueue::callbackOnMainThread, op, true);

which results in an EXC Bad Access at:

(obj->*method)(context);

Here's the traceback:

2013-05-29 16:16:06.135 [9521:f35f] MCIMAPSession.cc:816: select error : 0
2013-05-29 16:16:06.135 [9521:f35f] MCIMAPSession.cc:848: select ok
error: call to a function 'class_getName' that is not present in the target
(lldb) bt
* thread #1: tid = 0x2503, 0x0063006e, stop reason = EXC_BAD_ACCESS (code=2, address=0x63006e)
    frame #0: 0x0063006e
    frame #1: 0x002d7350 Mercurius`performOnMainThread(info=0x1ed5d700) + 92 at MCObject.cc:132
    frame #2: 0x002def22 Mercurius`-[LEPPPMainThreadCaller call](self=0x1ed51bd0, _cmd=0x386b321a) + 190 at MCMainThread.mm:29
    frame #3: 0x33b6b4a0 Foundation`__NSThreadPerformPerform + 460
    frame #4: 0x332288f6 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 14
    frame #5: 0x3322815c CoreFoundation`__CFRunLoopDoSources0 + 212
    frame #6: 0x33226f2e CoreFoundation`__CFRunLoopRun + 646
    frame #7: 0x3319a23c CoreFoundation`CFRunLoopRunSpecific + 356
    frame #8: 0x3319a0c8 CoreFoundation`CFRunLoopRunInMode + 104
    frame #9: 0x36d7833a GraphicsServices`GSEventRunModal + 74
    frame #10: 0x350b62b8 UIKit`UIApplicationMain + 1120
    frame #11: 0x000d7744 Mercurius`main(argc=1, argv=0x2fd2bd18) + 116 at main.m:16
    frame #12: 0x3b371b20 libdyld.dylib`start + 4

Any ideas what I'm doing wrong, or what's going on? I feel like I'm running into a wall here.

dinhvh commented 11 years ago

This line: MC_SAFE_RELEASE(uidSetResult); should be entirely removed in copyMessages().

ocrickard commented 11 years ago

Awesome, removed that line, and looks like it worked in fixing the memory issue.

However, I'm still getting the crash in op->callback()->operationFinished(op);

Is there an operationFinished function here?

dinhvh commented 11 years ago

What does op->callback() look like? Do you retain the objective-C operation?

ocrickard commented 11 years ago

Ahh! So it looks like there's a memory issue here. I didn't think I had to retain the operation because it's retained on its own in the parent's start method. I'll investigate later tonight or tomorrow morning and submit a pull request. Gotta run right now. Thanks for the help!

dinhvh commented 11 years ago

You're right. MCOOperation retains itself. It might be somewhere else.

dinhvh commented 11 years ago

Did you investigate what's going on? If you have a minimal code that can reproduce the issue, that could be great!

ocrickard commented 11 years ago

Yep, I've been investigating pretty much non-stop since yesterday. The good news is I think I understand your system much better now. Bad news is I'm still just as baffled by this bug. op->callback() is an OperationCallback.

Here's some code to replicate in the sample iOS project:

MCOIMAPCopyMessagesOperation *copyOperation = [_imapSession copyMessagesOperationWithFolder:@"INBOX" uids:[MCOIndexSet indexSetWithIndex:1] destFolder:@"[Gmail]/All Mail"];
    [copyOperation start:^(NSError *error, MCOIndexSet *destUIDs) {
        if(error) {
            NSLog(@"Error copying item:%@", error);
        } else {
            NSLog(@"Successfully copied item, new uids:%@", destUIDs);
        }
    }];
dinhvh commented 11 years ago

Did you get all the latest changes from the repository? I fixed the main issue yesterday.

When adding that in test-all.mm, I can't reproduce.

void testSMTPAgain() { MCOIMAPSession *session = [[MCOIMAPSession alloc] init]; session.username = @"xxxx"; session.password = @"xxxx"; session.hostname = @"imap.gmail.com"; session.port = 993; session.connectionType = MCOConnectionTypeTLS;

MCOIMAPCopyMessagesOperation copyOperation = [session copyMessagesOperationWithFolder:@"INBOX" uids:[MCOIndexSet indexSetWithIndex:116007] destFolder:@"[Gmail]/All Mail"]; [copyOperation start:^(NSError error, MCOIndexSet *destUIDs) { if(error) { NSLog(@"Error copying item:%@", error); } else { NSLog(@"Successfully copied item, new uids:%@", destUIDs); } }];

[[NSRunLoop currentRunLoop] run]; [session autorelease]; }

You have to call it in testAll().

Tested on iPhone Simulator and Mac OS X.

ocrickard commented 11 years ago

Awesome. I'll check this out when I get home. Thanks. I'm trying to get to the point of being self sufficient with this stuff, but clearly not there yet.

ocrickard commented 11 years ago

OK, just tested the newest code. It still results in an EXC_BAD_ACCESS, code 2:

void OperationQueue::callbackOnMainThread(Operation * op)
{
    if (op->isCancelled())
        return;

    if (op->callback() != NULL) {
        op->callback()->operationFinished(op); <----- TROUBLE
    }
}

This occurs on both the simulator, and on the device whenever I use the copy messages operation.

ocrickard commented 11 years ago

OK, so I just ran it in the test suite, and it works fine there. It only occurs when I add it to a running project.

So, take the exact same code, put it in the iOS UI Test somewhere, and it crashes.

ocrickard commented 11 years ago

Oh! I didn't understand that I had to use this line:

[[NSRunLoop currentRunLoop] run];

When I add it in to main-thread code, the error magically disappears. But when I use GCD to access the IMAP session off the main thread, even if I call run on the runLoop, it crashes. I don't fully understand the interaction between all of your code and the runLoop system here. My code uses GCD heavily to control access to resources, and do background NLP on the emails. Should I switch all my MailCore operation code to the main thread?

dinhvh commented 11 years ago

In an appkit app, you shouldn't have to run it yourself. I'll investigate what's going on when adding it to the example.

Hoa V. Dinh

On Thursday, May 30, 2013 at 5:27 PM, Oliver Clark Rickard wrote:

Oh! I didn't understand that I had to use this line: [[NSRunLoop currentRunLoop] run];
When I add it in to main-thread code, the error magically disappears. But when I use GCD to access the IMAP session off the main thread, even if I call run on the runLoop, it crashes. I don't fully understand the interaction between all of your code and the runLoop system here. My code uses GCD heavily to control access to resources, and do background NLP on the emails. Should I switch all my MailCore operation code to the main thread?

— Reply to this email directly or view it on GitHub (https://github.com/MailCore/mailcore2/issues/61#issuecomment-18716966).

dinhvh commented 11 years ago

Let me know if it's not fixed on your side.

ocrickard commented 11 years ago

Wow, works like a charm! Thanks!