User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.10 Safari/537.36 Steps to reproduce: fresh install Thunderbird 24.0b1.exe changed storage format to Maildir: Tools\Preference\Advanced\General\Config Editor Preferance name: mail.serverDefaultStoreContractID Value: @mozilla.org/msgstore/maildirstore;1 restarted Thunderbird imported my Outlook Express mails [Thunderbird-Tools-Import-Mail-Next-OutlookExpress] my Outlook Express email client has aprox 42000 emails organised in many subfolders the folder structure was inported in thunderbird no emails were displayed in thunderbird after import, but all emails are available on disk deleted *.msf files in order to rebuild all index files [msf] I created a global search [Body contains " " (space)] which was saved as Search Folder. click on the saved search folder this behaviour does not occur if thunderbird email format is mbox [mail.serverDefaultStoreContractID=@mozilla.org/msgstore/berkeleystore;1] Actual results: click on Search Folder thunderbitrd crashed Expected results: all existing email should be available in search folder all index files (*.msf) should be rebuilt

I imagine this is backend/db. Catalin is the only one crashing [@ MaildirStoreParser::ParseNextMessage(nsIFile*)] bp-39175622-150d-4011-bf81-a93232130723 0 xul.dll MaildirStoreParser::ParseNextMessage(nsIFile *) mailnews/local/src/nsMsgMaildirStore.cpp 1 xul.dll MaildirStoreParser::TimerCallback(nsITimer *,void *) mailnews/local/src/nsMsgMaildirStore.cpp 2 xul.dll nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp 3 xul.dll nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp 4 xul.dll nsThread::ProcessNextEvent(bool,bool *) xpcom/threads/nsThread.cpp 5 xul.dll NS_ProcessNextEvent(nsIThread *,bool) objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp 6 xul.dll mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) ipc/glue/MessagePump.cpp

well-known issue that m_db is null.

bp-93748b72-9dc8-4d79-b61d-b22dc2130817 (signature EMPTY) is also one of catalin's crashes.

(In reply to Makoto Kato (:m_kato) from comment #2) > well-known issue that m_db is null.

I think I am having the same problem here (both with thunderbird and icedove): Program received signal SIGSEGV, Segmentation fault. MaildirStoreParser::ParseNextMessage (this=this@entry=0x7fffb99d1a60, aFile= 0x7fffbabce8c0) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/local/src/nsMsgMaildirStore.cpp:1028 1028 rv = m_db->CreateNewHdr(nsMsgKey_None, getter_AddRefs(newMsgHdr)); (gdb) p m_db $11 = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>} #0 MaildirStoreParser::ParseNextMessage (this=this@entry=0x7fffb99d1a60, aFile=0x7fffbabce8c0) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/local/src/nsMsgMaildirStore.cpp:1028 #1 0x00007ffff32916be in MaildirStoreParser::TimerCallback ( aTimer=<optimized out>, aClosure=0x7fffb99d1a60) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/local/src/nsMsgMaildirStore.cpp:1106 #2 0x00007ffff373bf8a in nsTimerImpl::Fire (this=0x7fffbb5b5ec0) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/xpcom/threads/nsTimerImpl.cpp:543 #3 0x00007ffff373c05f in nsTimerEvent::Run (this=0x7fffffffca98) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/xpcom/threads/nsTimerImpl.cpp:627 #4 0x00007ffff37397cc in nsThread::ProcessNextEvent (this=0x7ffff6c26b60, mayWait=<optimized out>, result=0x7fffffffcccf) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/xpcom/threads/nsThread.cpp:626 #5 0x00007ffff370c654 in NS_ProcessNextEvent (thread=<optimized out>, mayWait=mayWait@entry=false) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/xpcom/build/nsThreadUtils.cpp:238 #6 0x00007ffff342d2e4 in mozilla::ipc::MessagePump::Run (this=0x7fffe565ddc0, aDelegate=0x7fffe564f240) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/ipc/glue/MessagePump.cpp:82 #7 0x00007ffff375949d in RunHandler (this=0x7fffe564f240) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/ipc/chromium/src/base/message_loop.cc:212 #8 MessageLoop::Run (this=0x7fffe564f240) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/ipc/chromium/src/base/message_loop.cc:186 #9 0x00007ffff31bb27f in nsBaseAppShell::Run (this=0x7fffde39b660) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/widget/xpwidgets/nsBaseAppShell.cpp:163 #10 0x00007ffff3090527 in nsAppStartup::Run (this=0x7fffdb3102e0) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/toolkit/components/startup/nsAppStartup.cpp:269 #11 0x00007ffff2774e71 in XREMain::XRE_mainRun (this=this@entry=0x7fffffffcef0) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/toolkit/xre/nsAppRunner.cpp:3856 #12 0x00007ffff2776e54 in XREMain::XRE_main (this=this@entry=0x7fffffffcef0, argc=argc@entry=1, argv=argv@entry=0x7fffffffe2d8, aAppData=aAppData@entry=0x7ffff6c23800) at /build/icedove-QEY8qr/icedove-24.6.0/mozilla/toolkit/xre/nsAppRunner.cpp:3924

Looking more into it, seems like ParseFolder is being called with a null mDatabase. In many places the code checks for mDatabase != nullptr but not in this specific code path: #0 nsMsgMaildirStore::RebuildIndex (this=<optimized out>, aFolder=0x7fffbf39d838, aMsgDB=0x0, aMsgWindow=<optimized out>, aListener=0x7fffbf39d848) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/local/src/nsMsgMaildirStore.cpp:1139 #1 0x00007ffff327e6a0 in nsMsgLocalMailFolder::ParseFolder ( this=0x7fffbf39d800, aMsgWindow=0x0, aListener=0x7fffbb0733e0) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/local/src/nsLocalMailFolder.cpp:197 #2 0x00007ffff3272a34 in nsMsgSearchOfflineMail::OpenSummaryFile ( this=0x7fffbb0733a0) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/base/search/src/nsMsgLocalSearch.cpp:292 #3 0x00007ffff3273da6 in nsMsgSearchOfflineMail::Search (this=0x7fffbb0733a0, aDone=0x7fffffffc8ee) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/base/search/src/nsMsgLocalSearch.cpp:707 #4 0x00007ffff32782a1 in nsMsgSearchSession::TimeSliceSerial ( this=this@entry=0x7fffba4e2c50, aDone=aDone@entry=0x7fffffffc8ee) at /build/icedove-QEY8qr/icedove-24.6.0/mailnews/base/search/src/nsMsgSearchSession.cpp:643 #5 0x00007ffff3278315 in nsMsgSearchSession::TimeSlice ( this=this@entry=0x7fffba4e2c50, aDone=aDone@entry=0x7fffffffc8ee)

I'm not really sure what the root cause of this is, but we should at least prevent the crash, and see if someone can show some other issues in parsing maildir folders.

So it looks like the m_db in MaildirStoreParser ultimately comes from here: <https://hg.mozilla.org/comm-central/annotate/4b97fba5d943/mailnews/local/src/nsLocalMailFolder.cpp#193>. This suggests that we're being asked to parse the folder without having an open database. I've been too long out of the database code to suggest why this scenario should be occurring, but I'm not sure wallpapering over it is necessarily the best idea.

OK I think you are correct. Looking at the equivalent mbox parser code, opening of the mailDB is a fairly involved process, including setting a backupDB (which was my change) See the code here: NS_IMETHODIMP nsMsgMailboxParser::OnStartRequest(nsIRequest *request, nsISupports *ctxt) So it looks like this may be another case of incomplete maildir implementation that I need to fix.

Comment on attachment 8543578 [details] [diff] [review] Check for null db Review of attachment 8543578 [details] [diff] [review]: ----------------------------------------------------------------- Cancelling review until the prior comment is addressed.

(In reply to Kent James (:rkent) from comment #9) > OK I think you are correct. Looking at the equivalent mbox parser code, > opening of the mailDB is a fairly involved process, including setting a > backupDB (which was my change) See the code here: > > NS_IMETHODIMP nsMsgMailboxParser::OnStartRequest(nsIRequest *request, > nsISupports *ctxt) > > So it looks like this may be another case of incomplete maildir > implementation that I need to fix. straightforward?

Hi Adam. I see you have this crash running 55.0b1. You might update your beta

Still a strong crash bp-b16ec38e-9161-419d-b405-d2cee0181121 64.0beta bp-3759e9ca-5bc3-4837-9117-12bb30181120 60.3.1

mangus, care do dust this one off?

I've rebased this and also taken out some URL nonsense. If it's OK to call with nullptr, there's no point to construct a URL for "mailbox://" which has no value. This needs more cleanup, since the 'delete parser' you can see there is also a foot gun. What you can't see in the patch is: MaildirStoreParser *parser = (MaildirStoreParser *) aClosure; So the TimerCallback() deleted the object it got given, which was created without ref-counting with this code: MaildirStoreParser *fileParser = new MaildirStoreParser(aFolder, aMsgDB, ... We'll save this for another bug.

Sorry, no review on time, so I'll take Kent's patch on my review and I'll add my patch separately.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/e756111cbbd0 fix maildir crash while parsing a folder. r=jorgk

Further down we already call parser->m_listener->OnStopRunningUrl(nullptr, ...);

Try is green. The code in question is triggered when a maildir folder is repaired. The OnStopRunningUrl() calls goes here: https://searchfox.org/comm-central/rev/85923675941e2dad376a1ecfdb26e3cf824ea7fc/mailnews/local/src/nsLocalMailFolder.cpp#3138 and later here: https://searchfox.org/comm-central/rev/85923675941e2dad376a1ecfdb26e3cf824ea7fc/mailnews/base/util/nsMsgDBFolder.cpp#1522 They all cope with null being passed in as the URL and I didn't see a spot where "mailbox://" would receive any useful processing. I've traced it down into nsresult nsMsgMailNewsUrl::UnRegisterListener() and the mUrlListeners.RemoveElement() returns false since apparently nothing was ever stored on the "mailbox://" URL which serves as a dummy. But hold on, if we pass null, this won't get executed: https://searchfox.org/comm-central/rev/85923675941e2dad376a1ecfdb26e3cf824ea7fc/mailnews/base/util/nsMsgDBFolder.cpp# NotifyFolderEvent(kFolderLoaded); That might have some undesired side-effects, one never knows in that crufty mailnews code. Let's not run any risk.

Comment on attachment 9032017 [details] [diff] [review] 906469-follow-up-URL-stuff.patch Withdrawing this, see comment #21.