Closed Bug 961564 Opened 6 years ago Closed 6 years ago

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

Categories

(MailNews Core :: Backend, defect, critical)

All
Linux
defect
Not set
critical

Tracking

(thunderbird29 fixed)

VERIFIED FIXED
Thunderbird 30.0
Tracking Status
thunderbird29 --- fixed

People

(Reporter: bc, Assigned: jcranmer)

References

Details

(4 keywords, Whiteboard: [regression:TB??])

Crash Data

Attachments

(2 files)

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.
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
Component: Folder and Message Lists → Backend
Product: Thunderbird → MailNews Core
Summary: null pointer crash in mozilla::mailnews::DecodedHeader(nsAString_internal const&) → null pointer crash in mozilla::mailnews::DecodedHeader(nsAString_internal const&) via nsMsgDBView::FetchAuthor
Summary: null pointer crash in mozilla::mailnews::DecodedHeader(nsAString_internal const&) via nsMsgDBView::FetchAuthor → Crash [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)]
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)
Flags: needinfo?(Pidgeot18)
Keywords: reproducible
Summary: Crash [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)] → Crash [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)] via nsMsgDBView::FetchAuthor
Whiteboard: [regression:TB??]
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?
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.
Flags: needinfo?(Pidgeot18)
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.
Crashed scrolling imap junk folder with ~1000 messages. bp-a510d1ad-30dc-4f0c-b35c-3050c2140206
Commented in bug 968155
Depends on: 968155
Duplicate of this bug: 968155
Crash Signature: [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)] → [@ mozilla::mailnews::DecodedHeader(nsAString_internal const&)] [@ nsMsgHeaderParser::ParseDecodedHeader(nsAString_internal const&, bool, unsigned int*, msgIAddressObject***)]
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
(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
Flags: needinfo?(Pidgeot18)
(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.
Flags: needinfo?(Pidgeot18)
jcranmer: attached. Let me know if you need anything else.

Gerv
Attached patch Simple fixSplinter Review
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.
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #8384175 - Flags: review?(irving)
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 <>
Attachment #8384175 - Flags: review?(irving) → review+
https://hg.mozilla.org/comm-central/rev/2b77b610f134
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 30.0
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.
Attachment #8384175 - Flags: approval-comm-aurora?
Attachment #8384175 - Flags: approval-comm-aurora? → approval-comm-aurora+
last trunk build crash 20140303030204
last aurora build crash 20140311004002
so v.fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.