Last Comment Bug 440236 - crash after connection lost [@ nsMsgDatabase::GetTableCreateIfMissing(char const*, char const*, nsIMdbTable**, unsigned int&, unsigned int&)], in v2 [@ nsMsgDatabase::GetTableCreateIfMissing]
: crash after connection lost [@ nsMsgDatabase::GetTableCreateIfMissing(char co...
Status: VERIFIED FIXED
: crash, fixed1.8.1.24, topcrash
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: unspecified
: x86 All
: -- critical (vote)
: Thunderbird 3.0rc1
Assigned To: David :Bienvenu
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-18 18:48 PDT by Wayne Mery (:wsmwk, NI for questions)
Modified: 2011-06-09 14:58 PDT (History)
4 users (show)
mozilla: blocking‑thunderbird3+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (1.55 KB, patch)
2009-10-14 14:02 PDT, David :Bienvenu
neil: review+
neil: superreview+
dveditz: approval1.8.1.next+
Details | Diff | Splinter Review

Description Wayne Mery (:wsmwk, NI for questions) 2008-06-18 18:48:23 PDT
crash after connection lost [@ nsMsgDatabase::GetTableCreateIfMissing]

TB44803148
Stack Signature	nsMsgDatabase::GetTableCreateIfMissing 25191134
Product ID	Thunderbird2
Build ID	2008042104
Trigger Time	2008-05-06 05:33:10.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	thunderbird.exe + (00519dcb)
URL visited	
User Comments	Just closing it. However, it was funny: my IMAP mail was unreachable via TB, while via Outlook there was little problem. Note: I lost the internet connection which was caused by my ADSL provider. PJTV
Since Last Crash	258177 sec
Total Uptime	258177 sec
Trigger Reason	Access violation
Source File, Line No.	e:/builds/tinderbox/Tb-Mozilla1.8-Release/WINNT_5.0_Depend/mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp, line 1569
Stack Trace 	
nsMsgDatabase::GetTableCreateIfMissing  [mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp, line 1569]
nsMailDatabase::GetAllOfflineOpsTable  [mozilla/mailnews/db/msgdb/src/nsMailDatabase.cpp, line 132]
nsImapMailFolder::UpdateImapMailboxInfo  [mozilla/mailnews/imap/src/nsImapMailFolder.cpp, line 2638]
XPTC_InvokeByIndex  [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
EventHandler  [mozilla/xpcom/proxy/src/nsProxyEvent.cpp, line 561]
nsMsgAccountManager::cleanupOnExit  [mozilla/mailnews/base/src/nsMsgAccountManager.cpp, line 1120]
hashEnumerate  [mozilla/xpcom/ds/nsHashtable.cpp, line 131]
nsHashtable::Enumerate  [mozilla/xpcom/ds/nsHashtable.cpp, line 319]
nsMsgAccountManager::CleanupOnExit  [mozilla/mailnews/base/src/nsMsgAccountManager.cpp, line 1634]
XPTC_InvokeByIndex  [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2169]
XPC_WN_CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1455]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1384]
js_Interpret  [mozilla/js/src/jsinterp.c, line 3955]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1403]
js_InternalInvoke  [mozilla/js/src/jsinterp.c, line 1478]
JS_CallFunctionValue  [mozilla/js/src/jsapi.c, line 4363]
nsJSContext::CallEventHandler  [mozilla/dom/src/base/nsJSEnvironment.cpp, line 1493]
nsJSEventListener::HandleEvent  [mozilla/dom/src/events/nsJSEventListener.cpp, line 195]
nsEventListenerManager::HandleEventSubType  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1655]
nsEventListenerManager::HandleEvent  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1762]
nsGlobalWindow::HandleDOMEvent  [mozilla/dom/src/base/nsGlobalWindow.cpp, line 1733]
DocumentViewerImpl::PageHide  [mozilla/layout/base/nsDocumentViewer.cpp, line 1215]
nsDocShell::FirePageHideNotification  [mozilla/docshell/base/nsDocShell.cpp, line 944]
nsDocShell::Destroy  [mozilla/docshell/base/nsDocShell.cpp, line 3586]
nsXULWindow::Destroy  [mozilla/xpfe/appshell/src/nsXULWindow.cpp, line 514]
nsWebShellWindow::Destroy  [mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 850]
nsWebShellWindow::HandleEvent  [mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 408]
nsWindow::DispatchEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 1319]
nsWindow::DispatchStandardEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 1359]
nsWindow::ProcessMessage  [mozilla/widget/src/windows/nsWindow.cpp, line 4509]
nsWindow::WindowProc  [mozilla/widget/src/windows/nsWindow.cpp, line 1507]
USER32.dll + 0x8724 (0x7e418724)
USER32.dll + 0x8806 (0x7e418806)
Comment 1 timeless 2008-06-19 09:40:50 PDT
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp&mark=1569,1682&rev=MOZILLA_1_8_BRANCH#1569

bienvenu, this code has always scared me....

is there a reason not to cache GetStore() once per function. And why do some people null check, but not everyone?
Comment 2 Wayne Mery (:wsmwk, NI for questions) 2009-10-05 17:15:26 PDT
bienvenu, question ...

(In reply to comment #1)
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp&mark=1569,1682&rev=MOZILLA_1_8_BRANCH#1569
> 
> bienvenu, this code has always scared me....
> 
> is there a reason not to cache GetStore() once per function. And why do some
> people null check, but not everyone?

#46 crash for 3.0b4. #67 for TB2.0.0.23
Comment 3 Wayne Mery (:wsmwk, NI for questions) 2009-10-05 17:20:43 PDT
(In reply to comment #2)
> bienvenu, question ...
> 
> (In reply to comment #1)
> > http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp&mark=1569,1682&rev=MOZILLA_1_8_BRANCH#1569
> > 
> > bienvenu, this code has always scared me....
> > 
> > is there a reason not to cache GetStore() once per function. And why do some
> > people null check, but not everyone?
> 
> #46 crash for 3.0b4. #67 for TB2.0.0.23

bp-8e1ce453-6ec2-40f4-b51e-cbb7d2081221 "era in standbay" (error in standby?)
bp-59727255-57b1-4927-b567-5ab272091003 "witching folder from left pane (smart folders). Clicking on a saved search including all messages (sent and received) from 3 accounts (8000 msgs)."
bp-6580fb7b-7330-49bc-9bec-a0c092090205 fails to process, but comment is "archiving of most emails in my quite large inbox and clicked on other folder in the folder panel -- the application archived for few minutes, five times I got message that folder_view (or something like that) script is taking too long, clicked 'continue' each time and it suddenly crashed."
Comment 4 David :Bienvenu 2009-10-05 17:21:59 PDT
I could never figure out how the db could be opened successfully without having a store - if we get an m_mdb, we should have a store. We can add more null checks, I suppose, though it always seems to be this CreateTableIfMissing that crashes...I can whip up a patch, I suppose.
Comment 5 David :Bienvenu 2009-10-14 13:23:23 PDT
This has become a top crash in 3.0 pre builds. Taking.
Comment 6 Wayne Mery (:wsmwk, NI for questions) 2009-10-14 13:51:37 PDT
and all OS
Comment 7 David :Bienvenu 2009-10-14 14:02:01 PDT
Created attachment 406297 [details] [diff] [review]
proposed fix

I've checked the callers and they all look like they handle the error returns. Since this is showing up in crash-stats, we should be able to see if this fix squashes the problem.
Comment 8 neil@parkwaycc.co.uk 2009-10-14 15:51:37 PDT
Comment on attachment 406297 [details] [diff] [review]
proposed fix

>+  mdb_err err  = m_mdbStore->StringToToken(GetEnv(), scope, &scopeToken);
>+  err = m_mdbStore->StringToToken(GetEnv(), kind, &kindToken);
Hmm, so we don't actually use err here?

>-    err = (nsresult) store->NewTable(GetEnv(), scopeToken,kindToken,
>+    err = (nsresult) m_mdbStore->NewTable(GetEnv(), scopeToken,kindToken,
>                                     PR_FALSE, nsnull, table);
Nit: could do with some whitespace cleanup.
[I'm confused. Does NewTable return an nsresult or an mdb_err?]
Comment 9 David :Bienvenu 2009-10-14 16:14:50 PDT
I removed the err stuff, and the confused cast, and fixed the whitespace...mdb used to return its own kind of errors, but I made them nsresults when I xpcom-ified the interface.
Comment 10 David :Bienvenu 2009-10-19 12:42:00 PDT
this is no longer showing up in crash-stats in recent builds.
Comment 11 Wayne Mery (:wsmwk, NI for questions) 2009-12-04 03:44:19 PST
(In reply to comment #10)
> this is no longer showing up in crash-stats in recent builds.

agree. last nightly build to crash is 2009100900, and nothing appears so far for 3.0
Comment 12 Wayne Mery (:wsmwk, NI for questions) 2010-01-16 03:37:22 PST
Bienvenu will this patch apply in v2?  
#11 crash in v2.0.0.23
Comment 13 David :Bienvenu 2010-01-19 17:35:52 PST
Comment on attachment 406297 [details] [diff] [review]
proposed fix

yes, it should apply to 1.8.1.next - it's basically a null check.
Comment 14 Daniel Veditz [:dveditz] 2010-02-05 11:54:42 PST
Comment on attachment 406297 [details] [diff] [review]
proposed fix

Approved for 1.8.1.24, a=dveditz
Comment 15 David :Bienvenu 2010-02-05 12:07:19 PST
fixed for 1.8.1.24

Note You need to log in before you can comment on or make changes to this bug.