797710 - Crash on message nsMsgFilterAfterTheFact::ApplyFilter, because of double free?

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Ludovic Hirlimann [:Usul]]

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

Using latest 18.0a1 x64 nightlies on Win7 x64...

Steps to reproduce the crash:

- manually create a simple message filter rule (for example: move messages that from/to/cc/bcc contains email from Inbox to certain subfolder under Local folders).
- save filter and execute it on Inbox.
- tb crashes.
- after restarting tb, the message is *copied* (instead of moved) in the folder specified in the filter. Original message remain in Inbox.

If the filter is left in place and an email arrives and is caught by the filter rule, tb crashes.

How can I troubleshoot this further?

PS: I'm using mail.serverDefaultStoreContractID @mozilla.org/msgstore/maildirstore;1 but I've had this with the default @mozilla.org/msgstore/berkeleystore;1 too. Don't know if that's useful info, but I thought I should mention it since it deviates from the default config.

I'm attaching links of related crashes from about:crashes...

Submitted Crash Reports

Report ID                                  Date Submitted
=============================================================
bp-345e1afc-ec8b-4b48-8b18-62e8a2121003    3/10/2012  1:00 pm
bp-c3c2b44d-0e67-4f0b-be76-77d942121003    3/10/2012  1:00 pm
bp-3d851308-0cb0-493c-b3fa-bc6432121003    3/10/2012  9:54 am
bp-e9f35f45-75c4-4114-a8ef-42a3b2120930    30/9/2012 10:45 pm
bp-8e4be6c0-0248-4390-8d68-c66382120930    30/9/2012  9:35 pm
bp-672327dc-ada5-4272-93b2-604e72120930    30/9/2012  9:34 pm
bp-f7494c6c-e0e6-4608-9ff0-0c8752120930    30/9/2012  1:33 pm
bp-9987858d-6b4a-4bdb-a229-5ca442120930    30/9/2012 12:51 pm
bp-22787d3c-b351-47b1-ac95-dadaf2120930    30/9/2012 12:45 μμ

Comment 1

[Ludovic Hirlimann [:Usul]]

0 	xul.dll 	nsMsgFilterAfterTheFact::ApplyFilter 	mailnews/base/search/src/nsMsgFilterService.cpp:538
1 	ntdll.dll 	LdrpAccessResourceData 	
2 	xul.dll 	CallWindowProcCrashProtected 	xpcom/base/nsCrashOnException.cpp:32
3 	xul.dll 	xul.dll@0x9be7b7 	
4 	xul.dll 	nsWindow::WindowProc 	widget/windows/nsWindow.cpp:4266
5 	user32.dll 	UserCallWinProcCheckWow 	
6 	xul.dll 	nsRefreshDriver::StopTimer 	layout/generic/nsTextFrameThebes.cpp:3334
7 	user32.dll 	UserCallWinProcCheckWow 	
8 	user32.dll 	UserCallWinProcCheckWow 	
9 	user32.dll 	DispatchMessageWorker 	
10 	user32.dll 	DispatchClientMessage 	
11 	xul.dll 	xul.dll@0x9bf51b 	
12 	user32.dll 	_fnDWORD 	
13 	xul.dll 	xul.dll@0x9bf51b 	
14 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:312
15 	xul.dll 	xul.dll@0x9bf51b 	
16 	xul.dll 	nsMsgFilterAfterTheFact::OnSearchDone 	mailnews/base/search/src/nsMsgFilterService.cpp:400
17 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:315
18 	xul.dll 	nsMsgSearchSession::NotifyListenersDone 	mailnews/base/search/src/nsMsgSearchSession.cpp:568
19 	xul.dll 	nsMsgSearchSession::TimerCallback 	mailnews/base/search/src/nsMsgSearchSession.cpp:504
20 	xul.dll 	xul.dll@0xb1d5bf 	
21 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:473
22 	xul.dll 	nsTimerEvent::Run 	xpcom/threads/nsTimerImpl.cpp:556
23 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:612
24 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:315
25 	xul.dll 	NS_ProcessNextEvent_P 	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:220
26 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:117
27 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:201
28 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:175
29 	xul.dll 	nsObserverList::AddObserver 	xpcom/ds/nsObserverList.cpp:19
30 	xul.dll 	nsThreadManager::GetCurrentThread 	xpcom/threads/nsThreadManager.cpp:186
31 	xul.dll 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:163
32 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:290
33 	xul.dll 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3782
34 	xul.dll 	nsLocalFile::Release 	xpcom/io/nsLocalFileWin.cpp:978
35 	xul.dll 	NS_TableDrivenQI 	objdir-tb/mozilla/xpcom/build/nsISupportsImpl.cpp:16
36 	xul.dll 	nsCOMPtr_base::assign_from_qi 	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp:56
37 	xul.dll 	ScopedXPCOMStartup::Initialize 	toolkit/xre/nsAppRunner.cpp:1181
38 	xul.dll 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3848
39 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3923

Comment 2

[Wayne Mery (:wsmwk)]

same question as in bug 797659, Klonos, can you reproduce this please using version 17 beta?

Comment 3

[Pieter Sybesma]

Only occuring when using filters on a maildir account

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to Pieter Sybesma from comment #3)
> Only occuring when using filters on a maildir account

Pieter, does that mean you can easily reproduce this crash ?

Comment 5

[Pieter Sybesma]

When i turn on the filters for the maildir account thunderbird crashes. Just tried it and it still happens.

https://crash-stats.mozilla.com/report/index/bp-1e0a97f0-b205-4847-9d12-b091b2130304

Comment 6

[Pieter Sybesma]

Messages are filtered to the destination folder. Have to do a repair folder to make them disappear from the inbox.

Mail filter used to crash Thunderbird is the following:

name="Vakantieveilingen-2"
enabled="no"
type="48"
action="Move to folder"
actionValue="mailbox://[e-mail]/Autocorrespondentie/Vakantievelingen"
condition="AND (from,is,[e-mail]) AND (subject,contains,Reservering Vakantieveilingen.nl)"

Comment 7

[Wayne Mery (:wsmwk)]

user jaise  also reports this with maildir
https://crash-stats.mozilla.com/report/index/e274e933-d079-4653-99f9-426c02140710
https://crash-stats.mozilla.com/report/index/13033a4b-40a1-43f5-be76-9c1be2140710
etc.

He writes...
2. create the filter "move"
3. Run the filter on folder
Then thunderbird crashes...But mails are moved properly.

Every time, after this, I need to repair inbox folder to correct msf file using folder property option. **Moved msg shows in inbox without body text**

If again run filter, msg copy to folder without body text & duplicate msg to the folder.

Comment 8

[Wayne Mery (:wsmwk)]

potential new topcrash. What might have changed in thunderbird 31 (or some intermediate version) to make this #9 ranked crash?  In version 24 it is pretty rare.

In version 31 about 60% are startup (less than one minute) with many being 10 seconds or less.
https://crash-stats.mozilla.com/report/list?signature=nsMsgFilterAfterTheFact%3A%3AApplyFilter%28bool*%29&product=Thunderbird&query_type=contains&range_unit=weeks&process_type=any&version=Thunderbird%3A31.0&version=Thunderbird%3A24.7.0&version=Thunderbird%3A24.6.0&version=Thunderbird%3A24.5.0&version=Thunderbird%3A34.0a1&version=Thunderbird%3A33.0a2&version=Thunderbird%3A32.0b1&version=Thunderbird%3A31.0b3&version=Thunderbird%3A31.0b2&hang_type=any&date=2014-08-20+11%3A00%3A00&range_value=4#tab-sigsummary

bp-5714ff1f-3770-4017-b5ef-ddd802140816
0 	xul.dll	nsMsgFilterAfterTheFact::ApplyFilter(bool*)	mailnews/base/search/src/nsMsgFilterService.cpp
1 	xul.dll	nsMsgApplyFiltersToMessages::RunNextFilter()	mailnews/base/search/src/nsMsgFilterService.cpp
2 	xul.dll	nsMsgFilterAfterTheFact::AdvanceToNextFolder()	mailnews/base/search/src/nsMsgFilterService.cpp
3 	xul.dll	nsMsgFilterService::ApplyFilters(int, nsIArray*, nsIMsgFolder*, nsIMsgWindow*)	mailnews/base/search/src/nsMsgFilterService.cpp
4 	xul.dll	nsMsgDBFolder::OnMessageClassified(char const*, unsigned int, unsigned int)	mailnews/base/util/nsMsgDBFolder.cpp
5 	xul.dll	nsMsgLocalMailFolder::OnMessageClassified(char const*, unsigned int, unsigned int)	mailnews/local/src/nsLocalMailFolder.cpp
6 	xul.dll	nsMsgDBFolder::CallFilterPlugins(nsIMsgWindow*, bool*)	mailnews/base/util/nsMsgDBFolder.cpp
7 	xul.dll	nsPop3Sink::EndMailDelivery(nsIPop3Protocol*)	mailnews/local/src/nsPop3Sink.cpp 

mconley@13187 947 m_curFilter->SetScope(nullptr);
beckley@143   948
beckley@143   949 if (m_searchHits.Length() > 0)
beckley@143   950  {
mwu@9431      951   bool applyMore = true;
beckley@143   952
kent@3238     953   m_nextAction = 0;
beckley@143   954   rv = ApplyFilter(&applyMore); 

kent@3330   539 // Tell postplugin filters if we are moving the message.
kent@3330   540 if (actionType == nsMsgFilterAction::MoveToFolder)
ehsan@13324 541   for (uint32_t i = 0; i < m_searchHits.Length(); i++)

Comment 9

[:aceman]

The original report states this also happens without maildir.
And what is the difference to bug 797712?

Comment 10

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #9)
> The original report states this also happens without maildir.
> And what is the difference to bug 797712?

I don't know whether there is a difference, technical  or otherwise. Ludo?

Comment 11

[Kent James (:rkent)]

The crash on m_searchHits is almost certainly caused by this method calling release twice on itself, as it is prone to do because the logic is quite convoluted. That was the subject of my ill-fated patch to Bug 695671 which still badly needs to be revived.

I don't have any idea what might have changed to increase this, but I think that whatever effort we expend should be to fix the logic of nsMsgFilterAfterTheFact so that, regardless of the error condition, we can guarantee that it will not call Release on itself multiple times.

Comment 13

[Tony Mechelynck [:tonymec]]

I've had many crashes while moving mail between folders or compacting folders since 16 september, starting a "wave of crashiness" with 7 crashes in approx. 2 weeks. Of these 7, #1 #2 and #7 were in nightlies (with symbols) with the same signature as this bug:
bp-9f6c18c2-963d-46d4-8fcb-308002140916
bp-34bf294f-0029-45a3-b9bf-979f92140920
bp-c42c85c1-67db-4a40-b705-667492141003
The others were in hourlies (with no symbols, and the crashreporter-symbols.zip files have already been scrapped), #3 and #4 at libxul.so+0xaeb783, #5 and #6 at libxul.so+0xa1c2f2, all four in build ID 20140922114952. Can't tell anymore if these symbol-less crashes were in the same vicinity.

Comment 14

[Tony Mechelynck [:tonymec]]

P.S. Forgot to say these crashes were all in SeaMonkey; the latest one of them in the build I'm still using ATM, namely:
Mozilla/5.0 (X11; Linux x86_64; rv:35.0) Gecko/20100101 Firefox/35.0 SeaMonkey/2.32a1 ID:20141003003005 c-c:71b770250d00 m-c:b85c260821ab

Comment 15

[Tony Mechelynck [:tonymec]]

I get this crash often, but not every time, and it seems to happen more often when moving many messages at a time or when compacting a large folder with many holes (hundreds of MB, while my mail.purge_threshhold_mb is set at 1).
Also, unlike the reporter, I'm on Linux.

Comment 16

[Tony Mechelynck [:tonymec]]

Hmm... Maybe I'm in the wrong bug? I followed Soccorro "Related bug" link.

Comment 17

[Tony Mechelynck [:tonymec]]

See also https://crash-stats.mozilla.com/report/list?signature=nsMsgFilterAfterTheFact%3A%3AApplyFilter%28bool*%29&range_value=14&range_unit=days&date=2014-10-03

Seen in Tb since 24.6.0 but with a sharp rise since 31.1.1; seen in Sm but sightings are too few for analysis (only 4 signatures have more than 1 sighting on Sm 2.32a1).

Comment 18

[Tony Mechelynck [:tonymec]]

Lengthening the period to 28 days shows 31.1.0 239, 31.1.1 602, 31.1.2 340, other versions less than 100 apiece for a grand total of 1341. Both i686 and x86_64 are present.

Comment 20

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk) from comment #8)
> potential new topcrash. What might have changed in thunderbird 31 (or some
> intermediate version) to make this #9 ranked crash?  In version 24 it is
> pretty rare.

perhaps the increase is due to Bug 860254 - Poison memory on free for all small allocations.
In TB31.3.0 the signature ranking is #7

Comment 21

[Kent James (:rkent)]

We're still seeing this in 38.2.0

I've resisted storing a variable that represents the need to do a release, figuring that in a double release that variable would also have been destroyed. But maybe that is too hasty. I've also got an ExQuilla user who is crashing quite regularly from this, so I might be able to get it tested.

I'll attach a simple patch that I want to try.

Comment 22

[Kent James (:rkent)]

Attachment: Keep a record of whether we have done the release

I've also added a MOZ_ASSERT so we might catch this in debug builds.

Comment 23

[[e-mail]]

Comment on attachment 8651347 [details] [diff] [review]
Keep a record of whether we have done the release

>+  if (mNeedsRelease)
>+  {
>+    Release(); // release ourselves.
>+    mNeedsRelease = false;
>+  }
>+  else {
>+    MOZ_ASSERT(false, "OnEndExecution called a second time");
>+  }
[I would have probably written this as:
  MOZ_ASSERT(mNeedsRelease, "OnEndExecution called a second time");
  if (mNeedsRelease)
  ...
]

Comment 24

[Pieter Sybesma]

Filtering on a maildir account works fine now (TB 38.2.0). Mail is moved to destination folder without crashing TB and disappears from the inbox. (Without repair folder :-)).
Thx.

Comment 25

[Kent James (:rkent)]

(In reply to Pieter Sybesma from comment #24)
> Filtering on a maildir account works fine now (TB 38.2.0). Mail is moved to
> destination folder without crashing TB and disappears from the inbox.
> (Without repair folder :-)).
> Thx.

Right, but that is due to other changes that were done in both maildir and nsMsgFilterAfterTheFact code. The random crash that this bug addresses continues to exist in various forms.

Comment 26

[Kent James (:rkent)]

Comment on attachment 8651347 [details] [diff] [review]
Keep a record of whether we have done the release

This patch was pushed (with suggested Neil edit) as http://hg.mozilla.org/releases/comm-esr38/rev/6e3195183fdd as an experimental build for a user test. Will be backed out after test.

Comment 27

[Kent James (:rkent)]

Backed out (as planned) https://hg.mozilla.org/releases/comm-esr38/rev/4679dfa6c00d

Comment 28

[Kent James (:rkent)]

Comment on attachment 8651347 [details] [diff] [review]
Keep a record of whether we have done the release

Pushed with suggested changes:

http://hg.mozilla.org/comm-central/rev/30dc87a51f94

Comment 29

[Kent James (:rkent)]

Comment on attachment 8651347 [details] [diff] [review]
Keep a record of whether we have done the release

http://hg.mozilla.org/releases/comm-aurora/rev/d65237cd6a9c
http://hg.mozilla.org/releases/comm-beta/rev/2fc38551ba84
http://hg.mozilla.org/releases/comm-esr38/rev/80d60cf515e5

Comment 30

[Wayne Mery (:wsmwk)]

This was uplifted to 38.3.0. per comment 29. Crashes are mostloy fixed in 38.3.0 And I believe this fixed bug 1163132 and bug 1153820. Doubtful the patch would help bug 537017 and bug 615928 in any way, or that there is even correlation.

However, signature is not totally gone. 
Some can be discounted as freak stacks: for example bp-e860eb86-a853-46c7-92f2-2b6c62151003 bp-40fd50de-6b87-4a22-bb45-1d83f2151002 
A couple have nsMsgSearchSession on the stack: bp-11c047b1-d350-4f75-ad05-757b42151201 (windows) bp-420cf6bf-9f49-4739-b2fb-e192c2151007 (Mac)

memory free related, but still exceedingly rare with je_free | nsMsgSearchOfflineMail::ProcessSearchTerm: bp-3d2495bb-70ed-403c-957b-ba5882151201 bp-3845701e-f86f-45bf-90ea-b3f4b2151008