1664858 - version 78 Crash in [@ MsgHdrsToTArray]

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

Our imap threading friend?

Crash report: https://crash-stats.mozilla.org/report/index/0264f555-6cb5-449f-9eaf-a7c040200914

Top 10 frames of crashing thread:

0 xul.dll MsgHdrsToTArray comm/mailnews/base/src/nsMsgUtils.cpp:1886
1 xul.dll nsImapMailFolder::OnStopRunningUrl comm/mailnews/imap/src/nsImapMailFolder.cpp:4939
2 xul.dll nsMsgMailNewsUrl::SetUrlState comm/mailnews/base/src/nsMsgMailNewsUrl.cpp:231
3 xul.dll nsImapMailFolder::SetUrlState comm/mailnews/imap/src/nsImapMailFolder.cpp:6337
4 xul.dll `anonymous namespace'::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, nsresult>::Run comm/mailnews/imap/src/nsSyncRunnableHelpers.cpp:221
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1211
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:501
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:109
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

Comment 1

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/releases/comm-esr78/file/tip/mailnews/base/src/nsMsgUtils.cpp#l1886
https://hg.mozilla.org/releases/comm-esr78/file/tip/mailnews/imap/src/nsImapMailFolder.cpp#l4939

MsgHdrsToTArray is getting null messages. I think we can treat that as empty and go on.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1664858_msghdrstoarray.patch

I wonder if this surfaced after bug 1658797.

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #2)

...
I wonder if this surfaced after bug 1658797.

I'd say both crashes started at the same time. Both first appear with 78.0b2 BETA - this bug with buildid 20200622204154 and bug 1658797 with buildid 20200616103211

So the regression is likely in the uplifts https://hg.mozilla.org/releases/comm-beta/pushloghtml?fromchange=THUNDERBIRD_78_0b1_RELEASE&tochange=THUNDERBIRD_78_0b2_RELEASE&full=1 If not that, then the merge for 78.0b1

Comment 4

[Ben Campbell]

Comment on attachment 9175705 [details] [diff] [review]
bug1664858_msghdrstoarray.patch

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

Yes, looks like my fault, and your patch looks sound to me.

Ideally, I'd like to go through and be really explicit about parameter validity in these sorts of cases (ie is it acceptable to pass in null? how about empty arrays?), but better to wait until the array-removal dust settles. The null-pointer problem tends go away with `nsTArray<>&` params, and I'd think empty arrays should perfectly valid (if a little pointless).
I shall take great pleasure in finally removing `MsgHdrsToTArray()` and the like.

Comment 5

[Wayne Mery (:wsmwk)]

It will be interesting to see if this kills it dead

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/250a6c111f71
fix Crash in [@ MsgHdrsToTArray]. r=benc DONTBUILD

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f51e43c76fbc
followup - allow null messages arg to nsImapMailFolder::InitCopyState. rs=test-bustage-fix

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 9175705 [details] [diff] [review]
bug1664858_msghdrstoarray.patch

[Approval Request Comment]
Safe crash fix, would take on 78:
https://hg.mozilla.org/comm-central/rev/250a6c111f71
https://hg.mozilla.org/comm-central/rev/f51e43c76fbc

Comment 9

[Wayne Mery (:wsmwk)]

Comment on attachment 9175705 [details] [diff] [review]
bug1664858_msghdrstoarray.patch

[Triage Comment]
Approved for esr78

Comment 10

[Rob Lemley [:rjl]]

Thunderbird 78.2.3:
https://hg.mozilla.org/releases/comm-esr78/rev/1ba7e22e39bc
https://hg.mozilla.org/releases/comm-esr78/rev/e4358982ebf4