633746 - crash [@ nsMsgDBView::SelectionChanged()] in quick filter or when using AttachmentExtractor extension

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ nsMsgDBView::SelectionChanged()]

most crashes such as bp-190d82af-b96c-49b8-b7a5-cd0d92110209 cite AttachmentExtractor addon  https://addons.mozilla.org/en-US/thunderbird/addon/attachmentextractor/
0	thunderbird.exe	nsMsgDBView::SelectionChanged	mailnews/base/src/nsMsgDBView.cpp:1110
1	thunderbird.exe	nsMsgDBView::ExpandAll	mailnews/base/src/nsMsgDBView.cpp:4720
2	thunderbird.exe	nsMsgThreadedDBView::InitSort	mailnews/base/src/nsMsgThreadedDBView.cpp:581
3	thunderbird.exe	nsMsgThreadedDBView::InitThreadedView	mailnews/base/src/nsMsgThreadedDBView.cpp:162
4	thunderbird.exe	nsMsgThreadedDBView::Open	mailnews/base/src/nsMsgThreadedDBView.cpp:95 

very few don't list the add-on...

bp-0f061365-e4ff-4dc7-baa9-5764b2110209
EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	thunderbird.exe	nsMsgDBView::SelectionChanged	mailnews/base/src/nsMsgDBView.cpp:1110
1	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
2	thunderbird.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2722
3	thunderbird.exe	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1740
4	js3250.dll	js_Invoke	js/src/jsinterp.cpp:1360 

and bp-d4fd6c36-c42c-445f-be7e-71e202110119

Comment 1

[Wayne Mery (:wsmwk)]

bp-0fb95cd5-be23-4907-9e97-9b12a2120514
also cites "Using attachment extractor with multiple feed folders selected, extracting all to a specific folder."
contacted author of addon

Comment 2

[Wayne Mery (:wsmwk)]

Andrew, does your development Version 1.4b1321741953 at https://addons.mozilla.org/en-us/thunderbird/addon/attachmentextractor/ address this crash problem?

bp-8ed46d8c-5443-4931-be6b-f4c662120813 is an example. addon verson 1.3.5.1

Comment 3

[Wayne Mery (:wsmwk)]

examined 15 crashes. most have AttachmentExtractor installed. highest version seen is 1.3.5.2

only 2-3 did NOT have AttachmentExtractor installed.

Comment 4

[Andrew Williamson [:eviljeff]]

Sorry, I'm not maintaining AttachmentExtractor any more.

1.4b1321741953 may fix this problem (I have it installed and don't experience any crashes) but it was never finished into a final release.  Its missing a number of the features in 1.3.* and is unlikely to pass the current AMO review standards.

Anyone is free to fork it to distribute a working and compliant version - I'm willing to make them co-owner of the add-on listing on AMO instead if they want also.

Comment 5

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 6

[Wayne Mery (:wsmwk)]

me bp-61d48827-fa84-4635-ba8e-50cbe2151008
 composing a message. I just went from compose window to do quick search, clicked and started typing there. 
 0 	xul.dll	nsMsgDBView::SelectionChanged()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgDBView.cpp:1194
1 	xul.dll	nsMsgDBView::CollapseAll()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgDBView.cpp:5241
2 	xul.dll	nsMsgDBView::DoCommand(int)	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgDBView.cpp:2668
3 	xul.dll	nsMsgQuickSearchDBView::DoCommand(int)	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgQuickSearchDBView.cpp:115
4 	xul.dll	NS_InvokeByIndex	xpcom/reflect/xptcall/md/win32/xptcinvoke.cpp

Comment 7

[Wayne Mery (:wsmwk)]

bclary "quick filtering" bp-41117c1f-44f4-4fe4-9ec7-86c0f2151008

A surprising number per week for nightly builds
bp-573bc650-8d6a-41c7-b2a3-e6d532151005
bp-402919cc-6c58-4447-9d16-d7fcc2151007
bp-0abaa342-72a6-47f1-b929-025102151005

Comment 8

[Magnus Melin [:mkmelin]]

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgDBView.cpp#1194

1194     nsresult rv = mTreeSelection->GetRangeAt(0, &startRange, &endRange);

mTreeSelection is null (GetSelectedIndices can still say we have a selection, the standalone msg win)

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: bug633746_selection_crash.patch

Comment 10

[Kent James (:rkent)]

Comment on attachment 8675083 [details] [diff] [review]
bug633746_selection_crash.patch

LGTM!

Comment 11

[Daniel Holbert [:dholbert]]

Just ran into what looks like this bug, in EarlyBird, after using quicksearch: bp-64f797d0-3703-4e64-893d-89cfc2151023

Magnus, is the patch ready to land? (& thanks for fixing!)

Comment 12

[Magnus Melin [:mkmelin]]

Yes, it's just waiting for an open tree (bug 1213839).

Comment 13

[Magnus Melin [:mkmelin]]

And bug 1213715

Comment 14

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin from comment #12)
> Yes, it's just waiting for an open tree (bug 1213839).

last night aleth wrote in irc "Tests are back, so the tree is now approval-required."

Comment 15

[Wayne Mery (:wsmwk)]

This morning I crashed bp-18060955-7086-418d-85e4-930812151104 but I don't recall having any quick filter active

Comment 16

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/073468887b0f -> FIXED

Comment 17

[Daniel Holbert [:dholbert]]

(Are you thinking of backporting this fix to Aurora (Earlybird), which just branched this past week? Seems like the recent spike in activity here (comment 6 onward) might've been from a recent regression, which will ride the trains for the 44 release-cycle if we don't backport this.)

Comment 18

[Wayne Mery (:wsmwk)]

44 will never "release". :) And it's only marginally a topcrash for 43.0a2 - not for 38.* nor anything else.  I'm not sure we have a regression, but I have no objections to uplifting to Aurora after this bakes well.

Comment 19

[Daniel Holbert [:dholbert]]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #18)
> 44 will never "release". :)

(But at least it does go to EarlyBird users, right? I'm on EarlyBird 44 right now, and I'd be more comfortable on that channel knowing that I've got a fix for this crash [once it's baked, as you say], since I've already tripped over it once.)

(Though I can see that there could be diminishing returns from backporting fixes to Aurora releases that are not destined to become official releases. So maybe it's not worth worrying about, dunno.)

Comment 20

[Ryan VanderMeulen [:RyanVM]]

As a TB beta user, I too would like to not have it be crashy when 43/44 hit that channel :). Looks like a pretty trivial patch to uplift anyway.

Comment 21

[Kent James (:rkent)]

Comment on attachment 8675083 [details] [diff] [review]
bug633746_selection_crash.patch

It's easy enough to flag this as something people care about for aurora and beta.

Comment 22

[Magnus Melin [:mkmelin]]

Yes should be backported.

Comment 23

[Kent James (:rkent)]

Comment on attachment 8675083 [details] [diff] [review]
bug633746_selection_crash.patch

https://hg.mozilla.org/releases/comm-beta/rev/0cdacb8632a1
http://hg.mozilla.org/releases/comm-aurora/rev/66acdae10268