Closed Bug 1350687 Opened 4 years ago Closed 4 years ago

Crash in nsPop3Protocol::Abort

Categories

(MailNews Core :: Networking: POP, defect)

Unspecified
Windows 8
defect
Not set
critical

Tracking

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

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

People

(Reporter: wsmwk, Assigned: jorgk-bmo)

References

Details

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

Crash Data

Attachments

(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:

  m_nsIPop3Sink->AbortMailDelivery(this);

One line fix: test m_nsIPop3Sink before dereferencing it. Leave it with me.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
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+
https://hg.mozilla.org/comm-central/rev/044bc24451d2870e57a6f2524914f98288c18627
Status: ASSIGNED → RESOLVED
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.