768480 - Mac OSX TB 13 crashes in nsMsgDBFolder::CreateFileForDB when going online. Caused by folder subscribed on server that no longer exists?

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[Stephan]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0.1
Build ID: 20120614114901

Steps to reproduce:

Wipe everything related to TB on the Mac (caches, libraries, profiles, etc).
Fresh-install TB 13.0.1 with no extension, nothing.
Start TB offline and configure an IMAP email account.
Go online.


Actual results:

TB crashes as soon as it goes online.

If TB starts in online mode, it displays the interface then crashes.
If TB starts offline, if works fine until I go online, then crashes.
Same in safe-mode.

Works if offline / not checking mails.
Unchecking "Enable Global Search and Indexer" in prefs does not make a difference.


Expected results:

Should not crash!
By the way, TB 13.0.1 on a Windows 7 works fine reading the same IMAP account.

I was told to "please also mention that crash signature NS_MsgHashIfNecessary is Mac-only" and report the crash ID bp-6a2e9a27-3fc7-4b30-868b-a364f2120626.

Comment 1

[Wayne Mery (:wsmwk)]

ref http://forums.mozillazine.org/viewtopic.php?f=39&t=2469047

according to crash stats, crashes are OS X 10.6 and 10.7.

And I find no windows equivalent crashes with NS_MsgHashIfNecessary, nsMsgDBFolder::CreateFileForDB or nsImapMailFolder::CreateClientSubfolderInfo in any crash signature

0	XUL	NS_MsgHashIfNecessary	
1	XUL	nsMsgDBFolder::CreateFileForDB	mailnews/base/util/nsMsgDBFolder.cpp:916
2	XUL	nsImapMailFolder::CreateClientSubfolderInfo	mailnews/imap/src/nsImapMailFolder.cpp:977
3	XUL	nsImapMailFolder::CreateClientSubfolderInfo	mailnews/imap/src/nsImapMailFolder.cpp:961
4	XUL	nsImapIncomingServer::PossibleImapMailbox	mailnews/imap/src/nsImapIncomingServer.cpp:1206
5	XUL	SyncRunnable4<nsIImapServerSink,const nsACString_internal&,char,int,bool*>::Run	mailnews/imap/src/nsSyncRunnableHelpers.cpp:244
6	XUL	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:657
7	XUL	NS_ProcessPendingEvents_P	objdir-tb/x86_64/mozilla/xpcom/build/nsThreadUtils.cpp:195
8	XUL	nsBaseAppShell::NativeEventCallback	widget/xpwidgets/nsBaseAppShell.cpp:130
9	XUL	nsAppShell::ProcessGeckoEvents	widget/cocoa/nsAppShell.mm:441
10	CoreFoundation	__CFRunLoopDoSources0	
11	CoreFoundation	__CFRunLoopRun	
12	CoreFoundation	CFRunLoopRunSpecific

Comment 2

[David :Bienvenu]

IMAP:5 protocol log would be helpful - https://wiki.mozilla.org/MailNews:Logging

presumably, the end of the log would show us what folder we were crashing on.

Comment 3

[Stephan]

Attachment: imap.log

As requested, attached is the imap.log corresponding to a crash.

Comment 4

[Stephan]

Looking at the log I thought, is that folder faulty? But, one has to remember that TB13 is quite happy with the very same folder on a Windows 7 PC.

Comment 5

[David :Bienvenu]

what if you unsubscribe from that folder (temporarily) on windows, and see if you can get your mac build to start.

Comment 6

[Stephan]

Went to my ISP webmail and... I was not subscribed to that folder? Yet I see it in TB on Windows, and TB on OSX tries to open it? So I subscribed and now the imap.log ends with:
428883968[118d1fb80]: ReadNextLine [stream=1f3d5510 nb=33 needmore=0]
428883968[118d1fb80]: 1f8af800:imap.online.net:A:CreateNewLineFromSocket: * LSUB () "/" "Urssaf, Assedic"
428883968[118d1fb80]: ReadNextLine [stream=1f3d5510 nb=36 needmore=0]
428883968[118d1fb80]: 1f8af800:imap.online.net:A:CreateNewLineFromSocket: * LSUB () "/" "Urssaf, Assedic///"

So the "Urssaf, Assedic" folder is the one I can subscribe/unsubscribe to, and the other one (ending with ///) a rogue folder coming from my ISP? I cannot unsubscribe from it, so it seems.

I should *not* crash TB (and it does not on Windows).
But I'll contact my ISP and ask them what's wrong with that folder.

Comment 7

[Stephan]

OK, there is this rogue "Urssaf, Assedic///" that the server thinks I'm subscribed to, even though it does not exist anymore. And I can not unsubscribe because it does not exist. Trying to sort out things with the ISP.

Nevertheless, it should not crash TB.

Comment 8

[Stephan]

The rogue folder has been deleted by the ISP and TB does not crash anymore. So that seem to have been the cause. Now, TB should not have crashed anyway.

Comment 9

[Wayne Mery (:wsmwk)]

bienvenu, wada, I find only Mac and linux crashes. previous patch with this signature is bug 562104

other examples: bp-e930ded8-3597-4d8f-80e8-fca5e2120630 bp-c0e6bf52-c69c-4e0e-a217-a5da72120613

Comment 10

[Wayne Mery (:wsmwk)]

This crash signature is gone in version 24.x.

New in version 24 is [@ NS_MsgHashIfNecessary(nsAutoString&) ]  however, none of the stacks I looked at contain nsMsgDBFolder::CreateFileForDB 

But perhaps the steps are still valid. SOmeone able to test on a Mac?

Comment 11

[Phoenix]

Resolved per whiteboard

Comment 12

[Wayne Mery (:wsmwk)]

It is back.

user in https://support.mozilla.org/en-US/questions/1038015
bp-3ab94152-f7ba-4514-9b9a-855de2141223
 0 	XUL	NS_MsgHashIfNecessary(nsAutoString&)	mailnews/base/util/nsMsgUtils.cpp
1 	XUL	nsMsgDBFolder::CreateFileForDB(nsAString_internal const&, nsIFile*, nsIFile**)	mailnews/base/util/nsMsgDBFolder.cpp
2 	XUL	nsImapMailFolder::CreateClientSubfolderInfo(nsACString_internal const&, char, int, bool)	mailnews/imap/src/nsImapMailFolder.cpp
3 	XUL	nsImapMailFolder::CreateClientSubfolderInfo(nsACString_internal const&, char, int, bool)	mailnews/imap/src/nsImapMailFolder.cpp
4 	XUL	nsImapIncomingServer::PossibleImapMailbox(nsACString_internal const&, char, int, bool*)	mailnews/imap/src/nsImapIncomingServer.cpp
5 	XUL	(anonymous namespace)::SyncRunnable4<nsIImapServerSink, nsACString_internal const&, char, int, bool*>::Run()	mailnews/imap/src/nsSyncRunnableHelpers.cpp 

http://hg.mozilla.org/releases/comm-esr31/annotate/7777d8694ab2/mailnews/imap/src/nsImapMailFolder.cpp#l942
 hg@0 	932  // Create an empty database for this mail folder, set its name from the user
hg@0 	933  nsCOMPtr<nsIMsgDatabase> mailDBFactory;
hg@0 	934  nsCOMPtr<nsIMsgFolder> child;
hg@0 	935
hg@0 	936  nsCOMPtr<nsIMsgDBService> msgDBService = do_GetService(NS_MSGDB_SERVICE_CONTRACTID, &rv);
hg@0 	937  NS_ENSURE_SUCCESS(rv, rv);
hg@0 	938  nsCOMPtr<nsIMsgDatabase> unusedDB;
geoff@12584 939 nsCOMPtr <nsIFile> dbFile;
hg@0 	940
hg@0 	941 // warning, path will be changed
bienvenu@1936 942 rv = CreateFileForDB(folderNameStr, path, getter_AddRefs(dbFile));


http://hg.mozilla.org/releases/comm-esr31/annotate/7777d8694ab2/mailnews/base/util/nsMsgDBFolder.cpp#l914
hg@0 	907 // path coming in is the root path without the leaf name,
hg@0 	908 // on the way out, it's the whole path.
geoff@12584 909 nsresult nsMsgDBFolder::CreateFileForDB(const nsAString& userLeafName, nsIFile *path, nsIFile **dbFile)
hg@0 	910 {
hg@0 	911    NS_ENSURE_ARG_POINTER(dbFile);
hg@0 	912
bienvenu@1936 	913 nsAutoString proposedDBName(userLeafName);
hg@0 	914  NS_MsgHashIfNecessary(proposedDBName);

Comment 13

[Kent James (:rkent)]

This code causes name[-1] to be read if the length of the passed string is zero. We should protect against this:

Comment 14

[Kent James (:rkent)]

Attachment: Check length of string before decrementing

I'm not sure if this is causing the crash or not, but this method should have protection against zero length string.

Comment 15

[[e-mail]]

Comment on attachment 8589203 [details] [diff] [review]
Check length of string before decrementing

* Protect both overloads for consistency. r=me with that fixed.
* Prefer to early return in the case of an empty string.
* Prefer use of IsEmpty() to check for empty string.

Comment 16

[Kent James (:rkent)]

Checked in https://hg.mozilla.org/comm-central/rev/4322a4f7e57a

We'll consider taking this simple patch in TB 38, also possibly TB 31.

Comment 17

[Kent James (:rkent)]

Comment on attachment 8589203 [details] [diff] [review]
Check length of string before decrementing

http://hg.mozilla.org/releases/comm-beta/rev/3f8f00759a24
http://hg.mozilla.org/releases/comm-aurora/rev/6c54d11a4ed5

Comment 18

[Wayne Mery (:wsmwk)]

Thanks for the patch. Crash is gone in version 31.7.0.
https://crash-stats.mozilla.com/search/?signature=~NS_MsgHashIfNecessary&product=Thunderbird&date=%3E2015-05-01&date=%3C2015-07-01&_facets=signature&_columns=date&_columns=signature&_columns=version&_columns=build_id&_columns=platform&_columns=uptime#crash-reports

also reported in https://support.mozilla.org/en-US/questions/1026271