1667120 - Crash in [@ nsImapProtocol::CreateNewLineFromSocket]

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect, P1)

Description

[Wayne Mery (:wsmwk)]

Priority issue.

For some odd reason, there are not beta crash reports - it jumped from alpha to release.
The earliest crash stack I find below is buildid 20200921105223 82.0a1

I personally crashed, thunderbird was unattended, a few hours after update bp-edc7031d-ae24-4879-a007-4cc910200924

82.0a1 Crash report: https://crash-stats.mozilla.org/report/index/a0e08f1c-a58b-48d0-996a-f42f80200921

Top 10 frames of crashing thread:

0 xul.dll nsImapProtocol::CreateNewLineFromSocket comm/mailnews/imap/src/nsImapProtocol.cpp:4820
1 xul.dll nsImapServerResponseParser::GetNextLineForParser comm/mailnews/imap/src/nsImapServerResponseParser.cpp:83
2 xul.dll nsIMAPGenericParser::AdvanceToNextToken comm/mailnews/imap/src/nsIMAPGenericParser.cpp:95
3 xul.dll nsImapServerResponseParser::ParseIMAPServerResponse comm/mailnews/imap/src/nsImapServerResponseParser.cpp:184
4 xul.dll nsImapProtocol::HandleIdleResponses comm/mailnews/imap/src/nsImapProtocol.cpp:1476
5 xul.dll nsImapProtocol::ImapThreadMainLoop comm/mailnews/imap/src/nsImapProtocol.cpp:1453
6 xul.dll nsImapProtocol::Run comm/mailnews/imap/src/nsImapProtocol.cpp:1116
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234
8 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:513
9 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:332

Comment 1

[Wayne Mery (:wsmwk)]

Please first assess quickly our best action to take in the next hours - do we backout, do we more forward with another (well proved) fix, etc ?

I am still assessing severity from the user side.

Comment 2

[Kai Engert [:KaiE:]]

This crashes when trying to dereference a pointer.

There's earlier code in the same function, which checks the function for non-null, prior to accessing it. So apparently that's a valid scenario, and we'd then crash.

It might be as simple as adding another null check? I'll suggest a patch.

Comment 3

[Kai Engert [:KaiE:]]

Attachment: 1667120-null.patch

Comment 4

[Kai Engert [:KaiE:]]

Comment on attachment 9177673 [details] [diff] [review]
1667120-null.patch

Asking Ben as alterantive reviewer, just in case he's around more quickly than Magnus.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9177673 [details] [diff] [review]
1667120-null.patch

Review of attachment 9177673 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, mailNewsUrl null is he problem so this should fix it. Thanks Kai! r=mkmelin

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dce522123b64
Prevent null pointer crash in nsImapProtocol::CreateNewLineFromSocket. r=mkmelin DONTBUILD

Comment 7

[Kai Engert [:KaiE:]]

Comment on attachment 9177673 [details] [diff] [review]
1667120-null.patch

[Approval Request Comment]
Regression caused by (bug #): 1590473
User impact if declined: crashing
Testing completed (on c-c, etc.): not yet
Risk to taking this patch (and alternatives if risky): no risk

Comment 8

[Rob Lemley [:rjl]]

Comment on attachment 9177673 [details] [diff] [review]
1667120-null.patch

[Triage Comment]
Approved by wsmwk via Matrix for 82.0b1

Comment 9

[Rob Lemley [:rjl]]

Thunderbird 82.0b1 build2:
https://hg.mozilla.org/releases/comm-beta/rev/50f1b08436b6

Comment 10

[Rob Lemley [:rjl]]

Comment on attachment 9177673 [details] [diff] [review]
1667120-null.patch

[Triage Comment]
For 78.3.0 top crash.

Comment 11

[Wayne Mery (:wsmwk)]

Thanks for fixing this so quickly

78.3.1 checkin
https://hg.mozilla.org/releases/comm-esr78/rev/00912779d73faef4277c57fe99aa5259b11d244d

crash-stats shows this is gone from both beta 82 and 78.3.1
https://crash-stats.mozilla.org/topcrashers/?product=Thunderbird&version=78.3.1
https://crash-stats.mozilla.org/topcrashers/?product=Thunderbird&version=78.3.1