Closed Bug 629497 Opened 13 years ago Closed 13 years ago

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

Categories

(MailNews Core :: Filters, defect)

x86
All
defect
Not set
critical

Tracking

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

RESOLVED FIXED
Thunderbird 6.0
Tracking Status
thunderbird3.1 --- .16-fixed
blocking-thunderbird3.0 --- -

People

(Reporter: wsmwk, Assigned: rkent)

References

()

Details

Crash Data

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #541879 +++
+++ This bug was initially created as a clone of bug 472019 +++

patch in bug 541879 landed for v3.1.5 but had little effect.  bienvenu writes: "It looks like somewhere between the check for the null database and the use of it, someone is closing the db. Reopening."

in http://getsatisfaction.com/mozilla_messaging/topics/message_filters-ixvhj#reply_3777630 user has a testcase of sorts to go with 
bp-43bef6d6-dba0-4ad2-a720-d809c2101021 (version 3.1.5). "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."

EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	thunderbird.exe	nsMsgDBFolder::CallFilterPlugins	mailnews/base/util/nsMsgDBFolder.cpp:2494
1	thunderbird.exe	nsMsgLocalMailFolder::UpdateFolder	mailnews/local/src/nsLocalMailFolder.cpp:637
2	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
3	thunderbird.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2722
4	thunderbird.exe	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1740
5	js3250.dll	js_Invoke	js/src/jsinterp.cpp:1360
6	js3250.dll	js_Interpret	js/src/jsops.cpp:2240
7	js3250.dll	js_Invoke	js/src/jsinterp.cpp:1368
8	js3250.dll	js_InternalInvoke	js/src/jsinterp.cpp:1423
9	js3250.dll	JS_CallFunctionValue	js/src/jsapi.cpp:5114
10	thunderbird.exe	nsJSContext::CallEventHandler	dom/base/nsJSEnvironment.cpp:2197
11	thunderbird.exe	nsJSEventListener::HandleEvent	dom/src/events/nsJSEventListener.cpp:269
12	thunderbird.exe	nsEventListenerManager::HandleEventSubType	content/events/src/nsEventListenerManager.cpp:1041
13	thunderbird.exe	nsEventListenerManager::HandleEvent	content/events/src/nsEventListenerManager.cpp:1147
14	thunderbird.exe	nsEventTargetChainItem::HandleEvent	content/events/src/nsEventDispatcher.cpp:246
15	thunderbird.exe	nsEventTargetChainItem::HandleEventTargetChain	content/events/src/nsEventDispatcher.cpp:310
16	thunderbird.exe	nsEventDispatcher::Dispatch	content/events/src/nsEventDispatcher.cpp:573
17	thunderbird.exe	nsEventDispatcher::DispatchDOMEvent	content/events/src/nsEventDispatcher.cpp:636


found one trunk crash from last 4 weeks
bp-fe3aa6b2-6358-4364-bc31-0db3a2110117
0	xul.dll	nsMsgDBFolder::CallFilterPlugins	mailnews/base/util/nsMsgDBFolder.cpp:2548
1	xul.dll	nsPop3Sink::EndMailDelivery	mailnews/local/src/nsPop3Sink.cpp:441
2	xul.dll	nsPop3Protocol::GetMsg	mailnews/local/src/nsPop3Protocol.cpp:2876
3	xul.dll	nsPop3Protocol::ProcessProtocolState	mailnews/local/src/nsPop3Protocol.cpp:3951
4	xul.dll	nsMsgProtocol::OnDataAvailable	mailnews/base/util/nsMsgProtocol.cpp:387
5	xul.dll	nsInputStreamPump::OnStateTransfer	netwerk/base/src/nsInputStreamPump.cpp:510
6	xul.dll	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:400
7	xul.dll	nsOutputStreamReadyEvent::Run	xpcom/io/nsStreamUtils.cpp:112
8	xul.dll	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:633
9	xul.dll	NS_ProcessNextEvent_P	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:250
10	xul.dll	nsBaseAppShell::Run	widget/src/xpwidgets/nsBaseAppShell.cpp:195
blocking-thunderbird3.1: --- → ?
haven't fully checked if this is related ...

nsCOMPtr<nsIJunkMailPlugin>::nsCOMPtr<nsIJunkMailPlugin>(nsQueryInterface) | nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)
bp-36d9fb0e-be33-4be8-8c76-a76482110121
EXCEPTION_ACCESS_VIOLATION_WRITE
0xffffffffe012f968
0	thunderbird.exe	nsCOMPtr<nsIJunkMailPlugin>::nsCOMPtr<nsIJunkMailPlugin>	objdir-tb/mozilla/dist/include/nsCOMPtr.h:572
1	thunderbird.exe	nsMsgDBFolder::CallFilterPlugins	mailnews/base/util/nsMsgDBFolder.cpp:2355
2	thunderbird.exe	nsPop3Sink::EndMailDelivery	mailnews/local/src/nsPop3Sink.cpp:441
3	thunderbird.exe	nsPop3Protocol::GetMsg	mailnews/local/src/nsPop3Protocol.cpp:2844
4	thunderbird.exe	nsPop3Protocol::ProcessProtocolState	mailnews/local/src/nsPop3Protocol.cpp:3930
5	thunderbird.exe	nsMsgProtocol::OnDataAvailable	mailnews/base/util/nsMsgProtocol.cpp:359
6	thunderbird.exe	nsInputStreamPump::OnStateTransfer	netwerk/base/src/nsInputStreamPump.cpp:510
7	thunderbird.exe	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:400
#21 crash for v3.1.9

bp-c3bb8fdb-a0c2-483b-9a59-2529b2110315 (alfred)
EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	thunderbird.exe	nsMsgDBFolder::CallFilterPlugins	mailnews/base/util/nsMsgDBFolder.cpp:2526
1	thunderbird.exe	nsPop3Sink::EndMailDelivery	mailnews/local/src/nsPop3Sink.cpp:441
2	thunderbird.exe	nsPop3Protocol::GetMsg	mailnews/local/src/nsPop3Protocol.cpp:2844
3	thunderbird.exe	nsPop3Protocol::ProcessProtocolState	mailnews/local/src/nsPop3Protocol.cpp:3930
4	thunderbird.exe	nsMsgProtocol::OnDataAvailable	mailnews/base/util/nsMsgProtocol.cpp:359
5	thunderbird.exe	nsInputStreamPump::OnStateTransfer	netwerk/base/src/nsInputStreamPump.cpp:510
6	thunderbird.exe	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:400 

bp-9a393254-7d89-4735-b5e9-e538e2110301 (g.danos)


is this crash the same?
 nsPop3Sink::EndMailDelivery(nsIPop3Protocol*) bp-822040fe-7a03-4d2a-b892-58c082110322
Summary: crash [@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)] → crash [@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)] | nsPop3Sink::EndMailDelivery
mDatabase is null, and the code style is depressing
bienvenu, rkent, would you be able to take a crack at this topcrash?

(In reply to comment #3)
> mDatabase is null, and the code style is depressing

and from bug 541879 comment 15
> It looks like somewhere between the check for the null database and the use of
> it, someone is closing the db. Reopening.

(I've not heard back from any crash reporters :( )
Crash Signature: [@ nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, int*)]
blocking-thunderbird3.1: ? → -
Flags: wanted-thunderbird+
Although we checked earlier in the code for a null database, one theory of this crash is that someone closes the database between that check and this call. This patch provides a just-in-time check for null database with graceful recovery.

The fix that really needs doing is to separate closing of the database from nulling the database pointer, which is a bigger project with lots of unforeseen consequences.

So while I am unenthusiastic about this fix, with both Standard8 and wsmwk encouraging action, I can't think of anything else to do.
Assignee: nobody → kent
Status: NEW → ASSIGNED
Attachment #538902 - Flags: review?(dbienvenu)
We don't know that anyone closed the db; someone could have nulled out the mDatabase pointer (e.g., folder.msgDatabase = null;) In which case, just holding and using a local reference to the db would probably be sufficient.

The only way I can see the db getting nulled out or closed is if a property change listener directly or indirectly does so. The junk status change also causes a folder event JunkStatusChanged to get sent, and there could be listeners for that as well that do interesting things, including re-entrantly calling into the backend c++ code. But I don't think we spin nested event loops so we shouldn't be executing any random events like timers.
Comment on attachment 538902 [details] [diff] [review]
Check YET AGAIN for null database

Given that we don't know what's causing this, I'll r+ this. (I.e., I don't know if using a local db comptr would fix it, and I don't want to experiment on our users in the releases). Can you add an assertion for mDatabase being non-null?.

Perhaps on the trunk, we could try using a local comptr instead of this fix and experiment on nightly users :-)
Attachment #538902 - Flags: review?(dbienvenu) → review+
Attached patch Fix to checkinSplinter Review
Checked in http://hg.mozilla.org/comm-central/rev/42afd587c7d2
Attachment #538902 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
the target fix version should be TB 5, right?
Target Milestone: --- → Thunderbird 6.0
rkent/bienvenu, this is still #9 crash for v3.1.15. Could you work up a patch to land for v3.1.16?
blocking-thunderbird3.1: - → ---
Is there any way that a bug like this can get officially requested for TB 3.1? The patch is trivial (except I'm not sure I have a 3.1 build anymore), it is the approval to land that is tricky.
Comment on attachment 540598 [details] [diff] [review]
Fix to checkin

requesting 3.1.16 approval, assuming 3.1.15 is out (I don't really know anymore)
Attachment #540598 - Flags: approval-thunderbird3.1.16?
Attachment #540598 - Flags: approval-thunderbird3.1.16? → approval-thunderbird3.1.16+
Though this is assigned to me, I am no longer organized to work with the TB 3.1 branch so I would appreciate if someone else could do the checkin.
yes, I'm in the middle of building it in preparation for landing it.
This looks like a null pointer de-reference to me, are we considering this as a security issues?
Depends on: 688135
despite this patch,  nsPop3Sink::EndMailDelivery for a current version is on the stack of bp-6c3c7a66-2eb8-400f-acc5-627fb2111223
0	xul.dll	nsMsgDBFolder::CallFilterPlugins	mailnews/base/util/nsMsgDBFolder.cpp:2676
1	xul.dll	nsPop3Sink::EndMailDelivery	mailnews/local/src/nsPop3Sink.cpp:441
2	xul.dll	nsPop3Protocol::GetMsg	mailnews/local/src/nsPop3Protocol.cpp:2912
3	xul.dll	nsPop3Protocol::ProcessProtocolState	mailnews/local/src/nsPop3Protocol.cpp:3987
4	xul.dll	nsMsgProtocol::OnDataAvailable	mailnews/base/util/nsMsgProtocol.cpp:387
5	xul.dll	nsInputStreamPump::OnStateTransfer	netwerk/base/src/nsInputStreamPump.cpp:510
6	xul.dll	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:400
You need to log in before you can comment on or make changes to this bug.