Closed Bug 541879 Opened 10 years ago Closed 9 years ago

crash [@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)]

Categories

(MailNews Core :: Filters, defect, critical)

x86
All
defect
Not set
critical

Tracking

(thunderbird3.1 .5-fixed, blocking-thunderbird3.0 -)

RESOLVED FIXED
Thunderbird 3.3a1
Tracking Status
thunderbird3.1 --- .5-fixed
blocking-thunderbird3.0 --- -

People

(Reporter: wsmwk, Assigned: Bienvenu)

References

()

Details

(Keywords: crash, topcrash, Whiteboard: [gs][gssolved])

Crash Data

Attachments

(2 files)

+++ This bug was initially created as a clone of bug 472019 +++

it's unclear what impact bug bug 472019 had on crash-stats - #37 crash for 3.0.0
[@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)] #64 crash for 3.0.1  

bp-16971c9b-b851-4c59-b69e-ddc172100124
0	thunderbird.exe	nsMsgDBFolder::CallFilterPlugins	 mailnews/base/util/nsMsgDBFolder.cpp:2511
1	thunderbird.exe	nsMsgLocalMailFolder::UpdateFolder	mailnews/local/src/nsLocalMailFolder.cpp:630
2	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
3	thunderbird.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2456
unlike bug 472019, this crash is all address 0x0
448 nsresult nsMsgLocalMailFolder::GetDatabase()
450   nsCOMPtr <nsIMsgDatabase> msgDB;
451   return GetDatabaseWOReparse(getter_AddRefs(msgDB));

455 NS_IMETHODIMP nsMsgLocalMailFolder::GetDatabaseWOReparse(nsIMsgDatabase **aDatabase)
461   nsresult rv = NS_OK;
464     rv = OpenDatabase();
471   NS_IF_ADDREF(*aDatabase = mDatabase);
472   return rv;

So this is how we get a null mDatabase and an NS_OK from GetDatabase()

2419   if (!mDatabase)
2420   {
2421     rv = GetDatabase();
2422     NS_ENSURE_SUCCESS(rv, rv);
2423   }

This is the guard which doesn't do the job it wanted.

2506   rv = mDatabase->GetNewList(&numNewKeys, &newKeys);

is the crash point on trunk.
Attached patch proposalSplinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #423320 - Flags: review?(bienvenu)
after several days of 3.0.1 being out, it seems OK to claim that none of the crashes seem to involve calendar. (vs  Bug 472019)

eg bp-16971c9b-b851-4c59-b69e-ddc172100124 no extension installed
Component: Backend → Filters
QA Contact: backend → filters
OS: Windows Vista → All
Attachment #423320 - Flags: review?(bienvenu) → review-
Comment on attachment 423320 [details] [diff] [review]
proposal

even though the callers ignore the return value, I'd rather not swallow the errors here. So I'd keep the NS_ENSURE_SUCCESS(rv, rv), and return NS_ERROR_NOT_AVAILABLE when we can't get the DB...
Looking at this at wsmwk's request, I did have one question. You should be able to rely on the rv for operations that initialize a pointer, otherwise we would need to check everywhere both the rv as well as the pointer, which is not really very pretty.

So don't you think that we should fix this at the upstream location nsMsgLocalMailFolder::GetDatabaseWOReparse as well? (I would still fix CallFilterPlugins since we can't be sure that we know the source of the null mDatabase).

Also, I've been using NS_ENSURE_STATE(mDatabase) in my own code for cases like this, which is a little shorter than NS_ENSURE_TRUE(mDatabase, NS_ERROR_NOT_AVAILABLE) which is what Bienvenu requested. Is there any issue with using that instead (which returns on error NS_ERROR_UNEXPECTED)?
bienvenu, see comment 6

timeless (or kent), pending bienvenu's response, can you do a revised patch?

#16 crash for v3.1.4 and ~#35 for v3.0.8, so => topcrash
(already has wanted for TB 3.1)
Keywords: topcrash
Whiteboard: [needs revised patch]
Attached patch proposed fixSplinter Review
this is what I had in mind.

yes, we should try to figure out who's not returning an error on a null db, but it's not clear to me where the underlying problem is (e.g., is it in ::OpenDatabase, or below?)
Assignee: timeless → bienvenu
Attachment #479810 - Flags: review?(kent)
Attachment #479810 - Flags: review?(kent) → review+
Attachment #479810 - Flags: approval-thunderbird3.1.5?
fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 479810 [details] [diff] [review]
proposed fix

a=Standard8. I'll land this with some others in a few mins.
Attachment #479810 - Flags: approval-thunderbird3.1.5? → approval-thunderbird3.1.5+
Checked in to 1.9.2 (sorry for forgetting to set the username):

http://hg.mozilla.org/releases/comm-1.9.2/rev/b8bf5511c02a
Target Milestone: --- → Thunderbird 3.3a1
I found only one report in getsatisfaction at http://gsfn.us/t/1g57q
Whiteboard: [needs revised patch] → [needs revised patch][gs]
Whiteboard: [needs revised patch][gs] → [gs]
posted followup qn to http://gsfn.us/t/1g57q
in http://getsatisfaction.com/mozilla_messaging/topics/message_filters-ixvhj#reply_3777630 user has a testcase of sorts to go with recent bp-43bef6d6-dba0-4ad2-a720-d809c2101021

"I still get the filter warning message every time I open one of the local folders that has new filtered messages, and have also had another crash on OKing the message. So apparently not resolved. 

Crash ID: bp-43bef6d6-dba0-4ad2-a720-d809c2101021 

Error mesasge: 
"One of your filters uses a custom header that contains an invalid character, such as ':', a non-printable character, a non-ascii character, or an eight-bit ascii character. Please edit the msgFilterRules.dat file, which contains your filters, to remove invalid characters from your custom headers." 

I've copied my msgFilterRules.dat below in case it gives any clues. 

version="9" 
logging="no" 
name="speleonics" 
enabled="yes" 
type="17" 
action="Move to folder" 
actionValue="mailbox://nobody@Local%20Folders/speleonics" 
condition="AND (all addresses,contains,speleonics)" 
name="Mathcad" 
enabled="yes" 
type="17" 
action="Move to folder" 
actionValue="mailbox://nobody@Local%20Folders/Mathcad" 
condition="AND (all addresses,contains,mathcad)" 
name="Magazine stuff" 
enabled="yes" 
type="17" 
action="Move to folder" 
actionValue="mailbox://nobody@Local%20Folders/Magazine%20renewals" 
condition="OR (from,contains,cieonline.co.uk) OR (from,contains,datateam.co.uk)" 
name="SPAM" 
enabled="yes" 
type="17" 
action="Move to folder" 
actionValue="mailbox://nobody@Local%20Folders/SPAM" 
condition="OR (subject,contains,-----SPAM-----) OR (subject,contains,***[SPAM]***)" 
name="MatlockFreeCycle" 
enabled="yes" 
type="17" 
action="Move to folder" 
actionValue="mailbox://nobody@Local%20Folders/Freecycle" 
condition="AND (all addresses,contains,MatlockFreeCycle)"
It looks like somewhere between the check for the null database and the use of it, someone is closing the db. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #15)
> It looks like somewhere between the check for the null database and the use of
> it, someone is closing the db. Reopening.

should thunderbird-3.1 .5-fixed be reset?
Really we should probably be filing a follow-up bug.
Depends on: 629497
(In reply to comment #17)
> Really we should probably be filing a follow-up bug.

Bug 629497
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Sorry, no, I can't. This crash seems to be 'non-repeatable'.

On 4/20/2011 15:01, Wayne Mery (vn) wrote:
> we could use your help.
> do you have a way to make this crash?
>
> On 1/27/2011 3:04 PM, Wayne Mery (vn) wrote:
>> see https://bugzilla.mozilla.org/show_bug.cgi?id=541879
>>
>> https://crash-stats.mozilla.com/report/index/bc671c31-b89c-4a33-8f5c-58aeb2101231
Crash Signature: [@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)]
Whiteboard: [gs] → [gs][gssolved]
You need to log in before you can comment on or make changes to this bug.