961564 - Crash [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)] via nsMsgDBView::FetchAuthor

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Bob Clary [:bc] (inactive)]

This bug was filed from the Socorro interface and is 
report bp-f99e185f-9ea6-4668-b984-1fea52140120.
=============================================================

Also bp-637e8bc6-218c-472e-a6c6-ed5c02140120

These are not necessarily the minimum steps but they reproduce well for me.

1. Open folder on imap server with > 1000 messages
2. Click and hold the arrow on the scrollbar in the message pane.
3. Drag the scrollbar either up or down
4. Crash.

Comment 1

[Wayne Mery (:wsmwk)]

no crashes prior to 20131213030204 build. so assume this is a regression

all crashes are linux except for one Mac. 


bp-f99e185f-9ea6-4668-b984-1fea52140120 linux

0	libxul.so	mozilla::mailnews::DecodedHeader(nsAString_internal const&)	objdir-tb/mozilla/dist/include/nsCharTraits.h
1	libxul.so	nsMsgDBView::FetchAuthor(nsIMsgDBHdr*, nsAString_internal&)	mailnews/base/src/nsMsgDBView.cpp
2	libxul.so	nsMsgDBView::CellTextForColumn(int, char16_t const*, nsAString_internal&)	mailnews/base/src/nsMsgDBView.cpp
3	libxul.so	nsMsgGroupView::CellTextForColumn(int, char16_t const*, nsAString_internal&)	mailnews/base/src/nsMsgGroupView.cpp
4	libxul.so	nsMsgDBView::GetCellText(int, nsITreeColumn*, nsAString_internal&)	mailnews/base/src/nsMsgDBView.cpp
5	libxul.so	nsTreeBodyFrame::PaintText(int, nsTreeColumn*, nsRect const&, nsPresContext*, nsRenderingContext&, nsRect const&, int&)	layout/xul/tree/nsTreeBodyFrame.cpp
6	libxul.so	nsTreeBodyFrame::PaintCell(int, nsTreeColumn*, nsRect const&, nsPresContext*, nsRenderingContext&, nsRect const&, int&, nsPoint)	layout/xul/tree/nsTreeBodyFrame.cpp
7	libxul.so	nsTreeBodyFrame::PaintRow(int, nsRect const&, nsPresContext*, nsRenderingContext&, nsRect const&, nsPoint)	layout/xul/tree/nsTreeBodyFrame.cpp
8	libxul.so	nsTreeBodyFrame::PaintTreeBody(nsRenderingContext&, nsRect const&, nsPoint)	layout/xul/tree/nsTreeBodyFrame.cpp
9	libxul.so	PaintTreeBody	layout/xul/tree/nsTreeBodyFrame.cpp
10	libxul.so	nsDisplayGeneric::Paint(nsDisplayListBuilder*, nsRenderingContext*)	layout/base/nsDisplayList.h
11	libxul.so	mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, nsIntRect const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, nsIntPoint const&, float, float, int)	layout/base/FrameLayerBuilder.cpp
12	libxul.so	mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)	layout/base/FrameLayerBuilder.cpp
13	libxul.so	mozilla::layers::BasicThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, mozilla::layers::DrawRegionClip, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*)	gfx/layers/basic/BasicThebesLayer.h
14	libxul.so	mozilla::layers::BasicThebesLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*)	gfx/layers/basic/BasicThebesLayer.cpp
15	libxul.so	mozilla::layers::BasicContainerLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*)	gfx/layers/basic/BasicContainerLayer.cpp
16	libxul.so	mozilla::layers::BasicLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags)	gfx/layers/basic/BasicLayerManager.cpp
17	libxul.so	nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const	layout/base/nsDisplayList.cpp
18	libxul.so	nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const	layout/base/nsDisplayList.cpp
19	libxul.so	nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int)	layout/base/nsLayoutUtils.cpp
20	libxul.so	PresShell::Paint(nsView*, nsRegion const&, unsigned int)	layout/base/nsPresShell.cpp
21	libxul.so	nsViewManager::ProcessPendingUpdatesForView(nsView*, bool)	view/src/nsViewManager.cpp
22	libxul.so	nsViewManager::ProcessPendingUpdates(nsViewManager::UpdatingMode)	view/src/nsViewManager.cpp
23	libxul.so	nsRefreshDriver::Tick(long, mozilla::TimeStamp)	layout/base/nsRefreshDriver.cpp
24	libxul.so	mozilla::RefreshDriverTimer::Tick()	layout/base/nsRefreshDriver.cpp
25	libxul.so	nsTimerImpl::Fire()	xpcom/threads/nsTimerImpl.cpp 

Pidgeot18@13371 398 nsString author;
Pidgeot18@13371 399 nsresult rv = aHdr->GetMime2DecodedAuthor(author);
Pidgeot18@13371 400
Pidgeot18@13371 401 nsCString emailAddress;
Pidgeot18@13371 402 nsString name;
Pidgeot18@13371 403 ExtractFirstAddress(DecodedHeader(author), name, emailAddress); 


Mac stack is slightly different bp-a1556da7-8ec5-4487-8e58-9237d2140106
0	XUL	mozilla::mailnews::DecodedHeader(nsAString_internal const&)	objdir-tb/x86_64/mozilla/dist/include/nsCharTraits.h
1	XUL	nsMsgDatabase::RowCellColumnToAddressCollationKey(nsIMdbRow*, unsigned int, unsigned char**, unsigned int*)	mailnews/db/msgdb/src/nsMsgDatabase.cpp
2	XUL	nsMsgDBView::GetCollationKey(nsIMsgDBHdr*, int, unsigned char**, unsigned int*, nsIMsgCustomColumnHandler*)	mailnews/base/src/nsMsgDBView.cpp
3	XUL	nsMsgDBView::Sort(int, int)	mailnews/base/src/nsMsgDBView.cpp
4	XUL	nsMsgThreadedDBView::Sort(int, int)	mailnews/base/src/nsMsgThreadedDBView.cpp

Comment 2

[Wayne Mery (:wsmwk)]

reproducible per Bob. regression of bug 842632?  
Ssorry, I don't currently have a regression range. Current best estimate is  early Jan. 


(Bob note nsMsgDBView::FetchAuthor has a purpose in the topic. please do not disturb)

Comment 3

[Bob Clary [:bc] (inactive)]

I keep crashing at this signature it wasn't showing up in the socorro related bugs. I guess removing the via wasn't necessary, but the other changes help it show up now.

bp-ef7f5f8b-b70a-4614-82f0-a979a2140130 and bp-666f7ce6-5854-475f-aeb3-dbb552140130 are my most recent crashes but they don't include FetchAuthor. Different bug?

Comment 4

[Joshua Cranmer [:jcranmer]]

I pushed <https://hg.mozilla.org/comm-central/rev/00d7b30ca499> on the 30th, which changes the method in question to no longer report the same code (although it may not have eliminated the crash, just moved it to a different signature). Offhand, I'd guess that what's going on is we have a null author somewhere; reimplementing nsIMsgHeaderParser in JS definitely catches this situation and avoids the crash.

Comment 5

[Bob Clary [:bc] (inactive)]

Just updated and it appears to help though I no longer have the imap folder with a huge number of entries. I'll keep an eye out for re-occurrences and will let you know if I see any more. Thanks, Daily had become unusable for me with this crash.

Comment 6

[Bob Clary [:bc] (inactive)]

Crashed scrolling imap junk folder with ~1000 messages. bp-a510d1ad-30dc-4f0c-b35c-3050c2140206
Commented in bug 968155

Comment 8

[Gervase Markham [:gerv]]

I can reproduce this 100% on my current junk folder (1023-1047 messages) using Earlybird 29.0a2 (2014-02-25). I just submitted 5 crash reports, e.g. 9d3975dc-8786-4e72-a09f-4c8482140226, 2ee85436-8c6a-4ac2-8f4a-527cf2140226. It happens when you scroll down to a certain point - presumably that point is the point which triggers the parser to parse the header of the message which causes it to crash. I can approach that point from either direction - there's definitely one message (subject probably starting "Ta...") which does it. 

I'm happy to supply a folder file if that will help anyone debug. But ask in the next couple of days, otherwise I'm going to clean out my spam.

Gerv

https://crash-stats.mozilla.com/report/index/9d3975dc-8786-4e72-a09f-4c8482140226
https://crash-stats.mozilla.com/report/index/2ee85436-8c6a-4ac2-8f4a-527cf2140226

Comment 9

[Wayne Mery (:wsmwk)]

(In reply to Joshua Cranmer [:jcranmer] from comment #4)
> I pushed <https://hg.mozilla.org/comm-central/rev/00d7b30ca499> on the 30th,
> which changes the method in question to no longer report the same code
> (although it may not have eliminated the crash, just moved it to a different
> signature). Offhand, I'd guess that what's going on is we have a null author
> somewhere; reimplementing nsIMsgHeaderParser in JS definitely catches this
> situation and avoids the crash.

Joshua, is this resolved then by jsmime 0.2, to happen (you are hoping) for TB30?  Do you want gerv's testcase?

Topcrash for Earlybird, so unless it's a crazy edge case we may want this fixed for the TB29+TB30 beta population

Comment 10

[Joshua Cranmer [:jcranmer]]

(In reply to Gervase Markham [:gerv] from comment #8)
> I'm happy to supply a folder file if that will help anyone debug. But ask in
> the next couple of days, otherwise I'm going to clean out my spam.

Just the .msf file would be sufficient.

Comment 11

[Gervase Markham [:gerv]]

Attachment: Gzipped .msf file from spam folder (spamcheck.msf.gz)

Comment 12

[Gervase Markham [:gerv]]

jcranmer: attached. Let me know if you need anything else.

Gerv

Comment 13

[Joshua Cranmer [:jcranmer]]

Attachment: Simple fix

Turns out that the problem wasn't quite what I thought it was--but I've fixed it anyways. It's a lot easier when you have debug symbols from a non-optimized build, instead of having to guess which inlined function invocation is causing the crash.

Comment 14

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 8384175 [details] [diff] [review]
Simple fix

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

Nice patch. For reference, the problem was with handling addresses like:

To: Descriptive Name <>

Comment 15

[Joshua Cranmer [:jcranmer]]

https://hg.mozilla.org/comm-central/rev/2b77b610f134

Comment 16

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8384175 [details] [diff] [review]
Simple fix

[Approval Request Comment]
Regression caused by (bug #): bug 842632
User impact if declined: Crashes if messages have certain malformed authors. Don't need to open the message, just scroll to it in the message pane (or sort by author).
Testing completed (on c-c, etc.): Has an attached test case. Will probably bake on c-c for a bit before being uplifted to c-a anyways.
Risk to taking this patch (and alternatives if risky): Extremely low to none.

Comment 17

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/43b1e7e0119c

Comment 18

[Wayne Mery (:wsmwk)]

last trunk build crash 20140303030204
last aurora build crash 20140311004002
so v.fixed