1400432 - Crash in nsImapCancelProxy::Run

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

Crash rate is low and doesn't happen with a recent build (most recent is bp-686a2b42-8bc7-42dc-8539-a628b0170912 	2017-09-12 56.0b3 buildid 20170819123318) but I'll forget about it later so it needs to be rechecked some period of time after nightly updates resume.

First crash on record in last 6 months is  bp-ed6c3706-e9b1-49bb-95d5-85c8d0170526 buildid 20170518114447 54.0b1
 0 	xul.dll	nsImapCancelProxy::Run()	C:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/imap/src/nsImapProtocol.cpp:1063
1 	xul.dll	nsThreadSyncDispatch::Run()	xpcom/threads/nsThreadSyncDispatch.h:38
2 	xul.dll	nsThread::ProcessNextEvent(bool, bool*)	xpcom/threads/nsThread.cpp:1264
3 	xul.dll	NS_ProcessNextEvent(nsIThread*, bool)	xpcom/threads/nsThreadUtils.cpp:389
4 	xul.dll	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)	ipc/glue/MessagePump.cpp:96

Comment 1

[Jorg K (CEST = GMT+2)]

class nsImapCancelProxy : public mozilla::Runnable {
public:
  nsImapCancelProxy(nsICancelable *aProxyRequest)
    : mozilla::Runnable("nsImapCancelProxy"), m_proxyRequest(aProxyRequest) {
  }
  NS_IMETHOD Run() {
    m_proxyRequest->Cancel(NS_BINDING_ABORTED); <==
    return NS_OK;
  }
private:
  nsCOMPtr<nsICancelable> m_proxyRequest;
};

should crash there since this is only called when 'm_proxyRequest' wasn't null.

Comment 2

[Wayne Mery (:wsmwk)]

Signature only exists in version 60, so appears to be a regression. bug 1344538?


All OS https://crash-stats.mozilla.com/signature/?product=Thunderbird&_sort=-date&version=60.0&version=60.0b10&version=60.0b11&version=60.0b9&signature=nsImapCancelProxy%3A%3ARun&date=%3E%3D2018-04-02T11%3A45%3A07.000Z&date=%3C2018-10-02T11%3A45%3A07.000Z

Comment 3

[Jorg K (CEST = GMT+2)]

I've checked the first tree reports and they all crash here:
  comm/mailnews/imap/src/nsImapProtocol.cpp:1040
which is the line shown in comment #1.

I'll submit a patch now.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1400432-proxy-crash.patch

I also renamed m_proxyRequest since the protocol already has a member with that name.

I'm not sure that it helps since this is only called if m_proxyRequest is already non-null. Perhaps Magnus can spot something.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9014102 [details] [diff] [review]
1400432-proxy-crash.patch

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

Let's try it. r=mkmelin

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/37c79adc0b98
Fix crash in nsImapCancelProxy::Run(). r=mkmelin

Comment 7

[Jorg K (CEST = GMT+2)]

Beta (TB 63):
https://hg.mozilla.org/releases/comm-beta/rev/1d73a395a1d45d7f9105909cf7e41082c92f11c9

Comment 8

[Jorg K (CEST = GMT+2)]

TB 60.3 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/f4245553ba1a7c8893f41ce8e0680af0703a9b7f

Comment 9

[Wayne Mery (:wsmwk)]

There are still crashes at line 1040. But perhaps a different cause?

bp-a8142738-59ac-4d86-b1ce-c527e0190806 60.8.0
bp-b5b253ed-11e6-4b17-96ea-8f98f0190717 60.7.2
bp-48641e7c-3dd2-464c-b7f8-46f1d0190614 60.7.0

Comment 10

[Jorg K (CEST = GMT+2)]

Well, we weren't sure whether the "fix" would help. It didn't.

  NS_IMETHOD Run() {
    if (mRequest)
1040   mRequest->Cancel(NS_BINDING_ABORTED);

So we know mRequest isn't null, but it's stale, so dereferencing crashes. That's harder to fix.

Comment 11

[Wayne Mery (:wsmwk)]

But no version 68 crashes.
Changing back to FIXED because of patch. (although unclear what we fixed)