Crash in nsXPCWrappedJS::Release sending mail (or saving to Sent or Drafts)

REOPENED
Unassigned

Status

defect
--
critical
REOPENED
5 months ago
17 days ago

People

(Reporter: wsmwk, Unassigned)

Tracking

(Depends on 1 bug, 4 keywords)

Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird_esr6065+ fixed, thunderbird65 affected, thunderbird66 affected)

Details

(Whiteboard: [regression:tb65], crash signature)

Attachments

(1 attachment)

Reporter

Description

5 months ago
Suddenly #1 crash for 65.0b1, and multiple users, so perhaps a regression.
I haven't checked to see if the cause is in smtp or imap.

This bug was filed from the Socorro interface and is
report bp-8d069688-7e3e-442d-847d-a0a0e0190103.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsXPCWrappedJS::Release js/xpconnect/src/XPCWrappedJS.cpp:266
1 xul.dll static unsigned long ReleaseThunk xpcom/reflect/xptcall/xptcall.cpp:28
2 xul.dll nsSmtpProtocol::~nsSmtpProtocol comm/mailnews/compose/src/nsSmtpProtocol.cpp:233
3 xul.dll void nsSmtpProtocol::~nsSmtpProtocol comm/mailnews/compose/src/nsSmtpProtocol.cpp:230
4 xul.dll nsDSURIContentListener::Release comm/mailnews/base/util/nsMsgProtocol.cpp:50
5 xul.dll [thunk]:nsImapProtocol::Release`adjustor{28}'  
6 xul.dll nsMsgComposeAndSend::~nsMsgComposeAndSend comm/mailnews/compose/src/nsMsgSend.cpp:329
7 xul.dll void nsMsgComposeAndSend::~nsMsgComposeAndSend comm/mailnews/compose/src/nsMsgSend.cpp:302
8 xul.dll nsMsgComposeAndSend::Release comm/mailnews/compose/src/nsMsgSend.cpp:260
9 xul.dll void CopyListener::~CopyListener comm/mailnews/compose/src/nsMsgCopy.cpp:49

=============================================================

bp-f82c1ee6-3822-4b81-b805-0b35c0190103 is also 65.0b1 and seems to be from another user.

and fwiw, crashes in other versions are comparatively rarer, with different stacks, like bp-59a43ef8-b0eb-44cc-957c-f98e80181206 60.3.2

Comment 1

5 months ago
Uninitialised and free'ed unconditionally is a sure recipe for disaster :-( - If for some reason the nsSmtpProtocol::Initialize() doesn't run, the DTOR will crash.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #9034549 - Flags: review?(mkmelin+mozilla)

Comment 2

5 months ago
Comment on attachment 9034549 [details] [diff] [review]
1517464-fix-crash.patch

Maybe handy for landing today, so first reviewer wins ;-)
Attachment #9034549 - Flags: review?(acelists)

Comment 3

5 months ago
Comment on attachment 9034549 [details] [diff] [review]
1517464-fix-crash.patch

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

It's a pity Initialize is separate from the constructor so that we can get partially initialized objects like this.
Attachment #9034549 - Flags: review?(acelists) → review+

Comment 4

5 months ago
Comment on attachment 9034549 [details] [diff] [review]
1517464-fix-crash.patch

We have a winner!
Attachment #9034549 - Flags: review?(mkmelin+mozilla)

Comment 5

5 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/03e39bc0bc42
Fix crash in SMTP DTOR by properly initialising/testing pointer. r=aceman
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED

Updated

5 months ago
Target Milestone: --- → Thunderbird 66.0

Updated

5 months ago
Attachment #9034549 - Flags: approval-comm-esr60+
Attachment #9034549 - Flags: approval-comm-beta+
Reporter

Comment 7

5 months ago
Thanks for the quick attention.

We will also want to monitor the changes to signature NS_CycleCollectorSuspect3, because it looks like 65.0b has a signficant bump in crash rate.
Reporter

Comment 9

4 months ago

It's a bit early to fully guage the state of 65.0b2 [1] but early results suggest this patch hasn't helped at all - the crash rate hasn't changed in either absolute numbers nor relative to other crash signatures - still ~30% of crashes. So more investigation is needed, and I think it is premature to uplift even a correctness patch to ESR - in part because the crash signature doesn't even exist in 60.4.0.

[1] the closest approximation to total crash count I can provide is

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter

Comment 10

4 months ago

Multiple examples of recent nightly crashes, for example bp-49ac9ce4-3c61-4e3d-a159-f5bdc0190122

Graph of beta https://crash-stats.mozilla.com/signature/?product=Thunderbird&release_channel=beta&signature=nsXPCWrappedJS%3A%3ARelease&date=%3E%3D2018-10-24T10%3A28%3A28.000Z&date=%3C2019-01-24T09%3A28%3A28.000Z#graphs

65.0b3 just shipped yesterday, so can't judge if it has the same crash rate. But no reason to expect it won't.

OS: Windows 10 → All
Whiteboard: [regression:tb65]
Reporter

Comment 11

4 months ago

statistically, patch definitely had no impact.

earliest crash for NS_CycleCollectorSuspect3 is buildid 20181111101548 65.0a1 bp-9cd3900b-1992-425b-b9a8-293960181112
(drum roll)
earliest crash for nsXPCWrappedJS::Release is buildid 20181110100128 65.0a1 bp-891ace88-c31e-410f-bae4-5fd580181218

Does any patch landing in early November time frame stand out as possible regressor?

Flags: needinfo?(jorgk)
Summary: Crash in nsXPCWrappedJS::Release sending mail (or saving to drafts?) → Crash in nsXPCWrappedJS::Release sending mail (or saving to Sent or Drafts)
Reporter

Updated

4 months ago
Crash Signature: [@ nsXPCWrappedJS::Release] → [@ nsXPCWrappedJS::Release] [@ NS_CycleCollectorSuspect3 ]

Comment 12

4 months ago

I think you're mixing up two issues here. The original crash in nsSmtpUrl::~nsSmtpUrl() and a crash coming from RDF:
comm/rdf/base/nsCompositeDataSource.cpp:515 - bp-9cd3900b-1992-425b-b9a8-293960181112

The latter will go when we eliminate RDF. No idea what caused the former. OK, the patch was a shot in the dark like so often, but it also didn't hurt and added some correctness.

Flags: needinfo?(jorgk)

Comment 13

2 months ago

FYI I have just encountered this crash in TB 66.0b3
bp-f43d1f2d-0bee-437a-a26b-1ea980190325

Comment 14

2 months ago

(In reply to Richard Leger from comment #13)

FYI I have just encountered this crash in TB 66.0b3
bp-f43d1f2d-0bee-437a-a26b-1ea980190325

Crashed occurred following those steps:

  1. Forward an email (with embedded pictures)
  2. Enter email address or recipient
  3. Click Send button
  4. Email send windows closed automatically
  5. Close a TB opened Tab (msg tab more likely) by pressing the cross close icon on the tab
    ==> TB crashed

After re-opening TB the sent message appeared correctly in sent items, and the tab I was trying to close may still be opened but I am not sure if it was the same tab or not... looks like it...

Comment 15

2 months ago

I tried that and it didn't crash, neither in TB 60 nor the forthcoming TB 67 beta.

Updated

a month ago
Assignee: jorgk → nobody
Reporter

Updated

17 days ago
Depends on: mail-killrdf
You need to log in before you can comment on or make changes to this bug.