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 624 forks source link

iOS: 50+ threads stuck on SMTPDisconnectOperation = out of memory = crash #1708

Open xaphod opened 6 years ago

xaphod commented 6 years ago

Mailcore2 totally rocks! It is super helpful, I really appreciate it being open-source.

I spent all day today going through this, and I cannot figure it out. What's happening since the latest update of my app: i'm seeing crash-reports from iOS 10.2 where there are 50+ threads all stuck on the same trace as below. It seems like whatever server the user is using is either taking forever to answer the SMTP bye command, or some TCP timeout is happening (it seems like mailcore's default there is 300 seconds, quite long).

screen shot 2018-02-14 at 4 45 21 pm

all 50+ threads the same as above

The issue is that during the time these threads are waiting, I believe they are holding onto the data in memory, ie MCOMessageBuilder.data(), which is large because it contains a JPG attachment (ie. can be 5MB). So since 50+ of these are hanging around, that's 250MB gone -- and my app is being killed / OOM'd.

@dinhviethoa can this be the case: that after the sendOperation is done, the attachment memory is still retained by the SMTPSession or some other object, until the SMTPDisconnect operation completes? ... I tried to figure it out myself, couldn't... Is it a bug that so many of these run in parallel like this?

--> it'd be awesome if the Data could be passed in as a file, NOT Data, so no memory is consumed until it is actually needed

Things I made sure of with the debugger:

My code:

        let builder = MCOMessageBuilder()
        builder.header.from = MCOAddress.init(displayName: nil, mailbox: smtpSession.username)
        builder.header.to = self.input.addresses.map { MCOAddress.init(mailbox: $0) }
        builder.header.subject = input.subject

        var body = input.body

        guard let data = ActionSerializer.shared.actionData(self) else {
            self.output.success = true // or it'll endlessly fire
            self.finishOperation()
            return
        }
        builder.addAttachment(MCOAttachment.init(data: data, filename: input.filename)) // BIG! 5MB or so
        builder.htmlBody = body

        guard let rfc822Data = builder.data(), let sendOperation = smtpSession.sendOperation(with: rfc822Data) else {
            assert(false, "ERROR")
            self.finishOperation()
            return
        }

        sendOperation.start() { (error) in
            if let error = error {
                WLog("SendEmailAction: ERROR sending email - \(error)")
            }
            self.finishOperation() // causes this entire scope to get deallocated (good)
        }

Many thanks again for Mailcore2, and any help would be super useful, thanks.

dinhvh commented 6 years ago

I think memory could be released before disconnecting with mailsmtp_quit().

You could add a MC_SAFE_RELEASE(mMessageData) under this line:

https://github.com/MailCore/mailcore2/blob/f708ce74e23b61ec6e5ae958eba0b8bcd8831a1e/src/async/smtp/MCSMTPSendWithDataOperation.cpp#L85

Let me know if it helps and then please send a pull request over if it's the case.

xaphod commented 6 years ago

Thanks for your amazingly quick & useful reply - I owe you 🍺🍺🍺 I'll get a build out with that fix, and if the crash doesn't come back, consider it fixed and send a pull request. Will take some time.