Closed Bug 1350687 Opened 4 years ago Closed 4 years ago

Crash in nsPop3Protocol::Abort


(MailNews Core :: Networking: POP, defect)

Windows 8
Not set


(thunderbird_esr52 affected, thunderbird53 affected, thunderbird54 fixed, thunderbird55 fixed)

Thunderbird 55.0
Tracking Status
thunderbird_esr52 --- affected
thunderbird53 --- affected
thunderbird54 --- fixed
thunderbird55 --- fixed


(Reporter: wsmwk, Assigned: jorgk-bmo)



(Keywords: crash, regression, Whiteboard: [regression:TB52?])

Crash Data


(1 file)

First seen in nightly 55.0a1 build 20170313030207 bp-c364efe9-9f32-4d1e-b24b-e314d2170314. Second in 54.0a2 build 20170314004004 bp-7085aa66-767f-4049-8db2-17a5c2170315. Which makes me wonder if this was the result of an uplift

0 	xul.dll	nsPop3Protocol::Abort()	C:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/mailnews/local/src/nsPop3Protocol.cpp:1039
1 	xul.dll	nsPop3Protocol::Cancel(nsresult)	C:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/mailnews/local/src/nsPop3Protocol.cpp:1057
2 	xul.dll	nsPop3Protocol::OnProxyAvailable(nsICancelable*, nsIChannel*, nsIProxyInfo*, nsresult)	C:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/mailnews/local/src/nsPop3Protocol.cpp:539
3 	xul.dll	mozilla::net::nsAsyncResolveRequest::DoCallback()	netwerk/base/nsProtocolProxyService.cpp:249
4 	xul.dll	mozilla::net::nsAsyncResolveRequest::OnQueryComplete(nsresult, nsCString const&, nsCString const&)	netwerk/base/nsProtocolProxyService.cpp:211
5 	xul.dll	mozilla::net::ExecuteCallback::Run()	netwerk/base/nsPACMan.cpp:81
6 	xul.dll	nsThread::ProcessNextEvent(bool, bool*)	xpcom/threads/nsThread.cpp:1264
7 	xul.dll	NS_ProcessNextEvent(nsIThread*, bool)	xpcom/threads/nsThreadUtils.cpp:389
8 	xul.dll	mozilla::gmp::GeckoMediaPluginServiceParent::Observe(nsISupports*, char const*, char16_t const*)	dom/media/gmp/GMPServiceParent.cpp:303
This was caused by bug 1344538, the "take 2" on the proxy issue. That was meant to fix the error handling, not produce a crash :-(

Look at the stack:
nsPop3Protocol::OnProxyAvailable() calls
  rv = LoadUrlInternal(m_url);
  if (NS_FAILED(rv)) {
    Cancel(rv); <=== 539

nsPop3Protocol::Cancel() calls

    m_proxyRequest = nullptr;

  Abort(); <==== 1057
  return nsMsgProtocol::Cancel(NS_BINDING_ABORTED);

void nsPop3Protocol::Abort() and that crashes:


One line fix: test m_nsIPop3Sink before dereferencing it. Leave it with me.
Assignee: nobody → jorgk
Looks link Cancel()/Abort() is now called in unexpected ways before the protocol is fully initialised. It would be nice to know how we get into this situation.
Attachment #8851370 - Flags: review?(rkent)
Comment on attachment 8851370 [details] [diff] [review]
1350687-pop-abort-crash.patch (v1).

Review of attachment 8851370 [details] [diff] [review]:

This seems pretty safe, though I wish I understood the design plan for shutting down a protocol better.
Attachment #8851370 - Flags: review?(rkent) → review+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
Comment on attachment 8851370 [details] [diff] [review]
1350687-pop-abort-crash.patch (v1).

Needs to get fixed in TB 54 (Aurora), too.
Attachment #8851370 - Flags: approval-comm-aurora+
Wayne, I'm sure you'll let me know how we fare with that fix. With Windows Dailies currently broken (bug 1352456), we might see something on Aurora.
Flags: needinfo?(vseerror)
Noted. Looks like we will need a few more days to be able to conclude it's effectiveness
Blocks: 1344538
Keywords: regression
No crashes for nightly builds after 20170329030228 and 54.0a2 builds after 20170330004005 (I wrote this a few days ago)
Flags: needinfo?(vseerror) → needinfo?(jorgk)
Thanks. No question here, so clearing NI.
Flags: needinfo?(jorgk)
Whiteboard: [regression:TB52?]
You need to log in before you can comment on or make changes to this bug.