Closed
Bug 752768
Opened 12 years ago
Closed 12 years ago
Ubuntu startup crash in nsMsgMailSession::OnItemAdded (sig: NS_ReadLine)
Categories
(MailNews Core :: Backend, defect)
Tracking
(thunderbird15-, thunderbird16- fixed, thunderbird17- fixed)
VERIFIED
FIXED
Thunderbird 18.0
People
(Reporter: wsmwk, Assigned: rkent)
References
(Blocks 1 open bug, )
Details
(Keywords: crash, topcrash, Whiteboard: [startupcrash][gs][regression: TB12] Maybe triggered by emails with French chars?)
Crash Data
Attachments
(1 file, 2 obsolete files)
5.59 KB,
patch
|
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-407e5914-e576-4934-a618-0aa152120504 . ============================================================= all crashes are version 12.0.1 0 libxul.so NS_ReadLine<char, nsIInputStream, nsCAutoString> nsReadLine.h:150 1 libxul.so MsgStreamMsgHeaders nsMsgUtils.cpp:2220 2 libxul.so nsMailboxService::StreamHeaders nsMailboxService.cpp:355 3 libxul.so libxul.so@0x100292b 4 libxul.so XPCWrappedNative::CallMethod XPCWrappedNative.cpp:2901 5 libxul.so XPC_WN_CallMethod XPCWrappedNativeJSOps.cpp:1542 6 libxul.so js::InvokeKernel jscntxtinlines.h:311 7 libxul.so js::Interpret jsinterp.cpp:2791 8 libxul.so js::InvokeKernel jsinterp.cpp:537 9 libxul.so js::Invoke jsinterp.h:157 10 libxul.so JS_CallFunctionValue jsapi.cpp:5449 11 libxul.so nsXPCWrappedJSClass::CallMethod XPCWrappedJSClass.cpp:1510 12 libxul.so nsXPCWrappedJS::CallMethod XPCWrappedJS.cpp:617 13 libxul.so PrepareAndDispatch xptcstubs_gcc_x86_unix.cpp:92 14 libxul.so nsMsgMailSession::OnItemAdded nsMsgMailSession.cpp:193 15 libxul.so nsMsgDBFolder::NotifyItemAdded nsMsgDBFolder.cpp:5004 selected comments I've just renamed a local folder I went offline, started compact all folders and have restarted afterwards. I went offline, compacted all folders and have restarted afterwards - problem seems not to occure if i stay offline!? I got to read an e-mail this time (last crash about 3 minutes ago). What's up with TB today?
Reporter | ||
Comment 1•12 years ago
|
||
#4 crasher for thunderbird 12.0.1, and #1 crash for linux https://crash-stats.mozilla.com/report/list?version=Thunderbird%3A12.0.1&query_search=signature&query_type=contains&reason_type=contains&date=2012-05-25&range_value=28&range_unit=days&hang_type=any&process_type=any&signature=NS_ReadLine%3Cchar%2C%20nsIInputStream%2C%20nsCAutoString%3E
Keywords: topcrash
Summary: startup crash in NS_ReadLine → Ubuntu startup crash in NS_ReadLine
Whiteboard: [startupcrash]
Reporter | ||
Comment 2•12 years ago
|
||
(see if socorro picks up signature @ NS_ReadLine. doubtful)
Crash Signature: [@ NS_ReadLine<char, nsIInputStream, nsCAutoString>] → [@ NS_ReadLine<char, nsIInputStream, nsCAutoString>]
[@ NS_ReadLine]
Comment 3•12 years ago
|
||
A friend of mine (not computer expert) is seeing this on this Ubuntu 12.04 machine, too.
> I've just renamed a local folder
We did the same.
Comment 4•12 years ago
|
||
Using the official Thunderbird 13 x64 works http://releases.mozilla.org/pub/mozilla.org/thunderbird/releases/13.0.1/linux-x86_64/en-GB/
Comment 5•12 years ago
|
||
Could somebody please report this back to Ubuntu with high priority?
Comment 6•12 years ago
|
||
also reported here: http://forum.ubuntu-fr.org/viewtopic.php?pid=9720811
Comment 7•12 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #5) > Could somebody please report this back to Ubuntu with high priority? Done: https://bugs.launchpad.net/thunderbird/+bug/1016818
Updated•12 years ago
|
See Also: → https://launchpad.net/bugs/1016818
Comment 8•12 years ago
|
||
That launchpad bug was for @ nsResProtocolHandler::nsResProtocolHandler
See Also: https://launchpad.net/bugs/1016818 →
Comment 9•12 years ago
|
||
Another user report the problem here (on Geckozone): http://www.geckozone.org/forum/viewtopic.php?f=4&t=106780&p=698304&e=698304 Is there anything that can be done to help you understand the issue?
Comment 10•12 years ago
|
||
A nother one in Ubuntu french forum. http://forum.ubuntu-fr.org/viewtopic.php?pid=10388521#p10388521 https://crash-stats.mozilla.com/report/index/bp-24137983-cfe5-4d6c-93e3-e70292120813 It's only for french user ??
Comment 11•12 years ago
|
||
My friend from comment 3 also is located in France, but he's English and I think using the English locale in Ubuntu (OS and user account). He's using a French keyboard, Paris time, and Orange as ISP. The crash is in nsMsgDBFolder::NotifyItemAdded(), so my guess would be that the trigger are French emails (of which he also gets a lot, e.g. from LeMonde) with French accents and these chars in the emails are triggering the crash.
Updated•12 years ago
|
Whiteboard: [startupcrash] → [startupcrash] Maybe triggered by emails with French chars?
Reporter | ||
Comment 12•12 years ago
|
||
hmm. I haven't found the windows equivalent signature. No crash sigs contain nsMsgMailSession::OnItemAdded. (There are crashes for the frame below, nsMsgDBFolder::NotifyItemAdded. but they are not startup - for example nsMsgDBFolder::NotifyItemAdded(nsISupports*) 2febe726-4c20-4adc-84ae-b113d2120829 ) first crash is a single TB12 beta build 20120315033921 bp-5f9865eb-73a8-42d7-9524-e8a9b2120322 Not strictly match to bug 124287's title, but WTH
Blocks: folders-with-special-characters
Summary: Ubuntu startup crash in NS_ReadLine → Ubuntu startup crash in nsMsgMailSession::OnItemAdded (sig: NS_ReadLine)
Whiteboard: [startupcrash] Maybe triggered by emails with French chars? → [startupcrash][gs][regression: TB12] Maybe triggered by emails with French chars?
Reporter | ||
Comment 13•12 years ago
|
||
gee, no solid progress on this topcrash regression?
Assignee | ||
Comment 14•12 years ago
|
||
AFAICT streamHeaders is not used in the core code, and was originally added to support extensions (primarily Conversations) It also appears from the crash dump to be coming from a javascript user. Looking at the extension list on the crash reports, I see at least one that claims no extensions loaded, which confuses me. But still, there is no error checking in the call to NS_ReadLine, and the likely failure is a null aStream here: 116 nsresult rv = aStream->Read(aBuffer->buf, kLineBufferSize, &bytesRead); Looking at where that comes from, in nsMailboxService.cpp we have 321 rv = folder->GetOfflineFileStream(msgKey, &messageOffset, &messageSize, getter_AddRefs(inputStream)); 322 NS_ENSURE_SUCCESS(rv, rv); 323 return MsgStreamMsgHeaders(inputStream, aConsumer); Is the rv test enough? No, because in GetOfflineFileStream we have: nsresult rv = GetDatabase(); NS_ENSURE_SUCCESS(rv, NS_OK); That is, any failure to get the database returns OK but a null filestream. This is exactly why I dislike using NS_ENSURE_SUCCESS(rv, rv) when what I really care about is a non-null pointer. In my own code, I would have used instead: NS_ENSURE_STATE(inputStream) and this would have been prevented. I'll take this and do a fix.
Assignee | ||
Updated•12 years ago
|
QA Contact: kent
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → kent
Status: NEW → ASSIGNED
Assignee | ||
Updated•12 years ago
|
QA Contact: kent
Assignee | ||
Comment 15•12 years ago
|
||
This xpcshell test generates the crash. Fix to follow.
Assignee | ||
Comment 16•12 years ago
|
||
Jonathan, in your Conversations addon you disable the call to streamHeaders. Is it because of the crash that occurs here?
Assignee | ||
Comment 17•12 years ago
|
||
Attachment #660626 -
Attachment is obsolete: true
Attachment #660636 -
Flags: review?(irving)
Comment 18•12 years ago
|
||
I disable the call to streamHeaders because that function just doesn't work: it sometimes returns part of the header, and the first few bytes of the headers are missing. I once tried to debug this with Bienvenu but we didn't get far. If you were to find out what's happening, that would be fantastic :)
Comment 19•12 years ago
|
||
Oh, my, I was scratching my head whilst looking at this yesterday because I thought the crash was happening here: http://hg.mozilla.org/mozilla-central/file/f36b93c70d05/netwerk/base/public/nsReadLine.h#l150 ... based on the line numbers in https://crash-stats.mozilla.com/report/index/407e5914-e576-4934-a618-0aa152120504. Now I've just realized that report is from an old version before the license header change ;) Anyway, thanks for spotting that. Incidentally, I have an addon which calls streamHeaders too...
Comment 20•12 years ago
|
||
> GetOfflineFileStream we have: > nsresult rv = GetDatabase(); > NS_ENSURE_SUCCESS(rv, NS_OK); > That is, any failure to get the database returns OK but a null filestream. That's either just wrong, a bug in GetDatabase(), or - if it's clearly documented -, a mean API that the caller here misunderstood. If the latter, please fix GetDatabase(), not the caller. > AFAICT streamHeaders is not used in the core code FYI, I personally saw the bug on a machine with a fresh installation of Ubuntu and Ubuntu's Thunderbird package. The only extensions were those added automatically by Ubuntu. So, these would be a place to look at.
Comment 21•12 years ago
|
||
Chris, I've had consistently wrong results with streamHeaders, I've tried debugging these for a while, and I couldn't get anywhere. Looks like the backend gets the offset of the message wrong and starts streaming at a wrong offset...
Reporter | ||
Comment 22•12 years ago
|
||
protz, is there a bug report on streamHeaders?
Comment 23•12 years ago
|
||
(In reply to Jonathan Protzenko [:protz] from comment #21) > I've had consistently wrong results with streamHeaders ... > starts streaming at a wrong offset... Oh, that's interesting! This might be the cause of bug 790912.
Assignee | ||
Comment 24•12 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #20) > > GetOfflineFileStream we have: > > nsresult rv = GetDatabase(); > > NS_ENSURE_SUCCESS(rv, NS_OK); > > That is, any failure to get the database returns OK but a null filestream. > > That's either just wrong, a bug in GetDatabase(), or - if it's clearly > documented -, a mean API that the caller here misunderstood. If the latter, > please fix GetDatabase(), not the caller. For a crash fix like this that I hope to get landed in a .0.* version, I tend to be extremely cautious in what I change. Because GetOfflineFileStream is used in places other than streamHeader, I am reluctant to change its behavior in a .0.* version. I agree though that the NS_ENSURE_SUCCESS(rv, NS_OK) violates expectations and should be changed, just not in this patch. > > > AFAICT streamHeaders is not used in the core code > > FYI, I personally saw the bug on a machine with a fresh installation of > Ubuntu and Ubuntu's Thunderbird package. As I mentioned, I also noted crash reports with no extensions installed, so I am also concerned here. But the issue I have identified in the patch is real, the unit test crashes with the correct signature without the fix, so this patch should be landed IMHO.
Assignee | ||
Updated•12 years ago
|
Comment 25•12 years ago
|
||
Comment on attachment 660636 [details] [diff] [review] Rev2: with crash fix Review of attachment 660636 [details] [diff] [review]: ----------------------------------------------------------------- The patch looks fine. We should land this, but I'd like to keep either this or a related bug open to understand why we're failing to open the offline stream in these cases and make GetOfflineFileStream behave better.
Attachment #660636 -
Flags: review?(irving) → review+
Assignee | ||
Comment 26•12 years ago
|
||
Checked in https://hg.mozilla.org/comm-central/rev/c05d43695bef
Attachment #660636 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Assignee: kent → nobody
Component: General → Backend
Flags: in-testsuite+
Product: Thunderbird → MailNews Core
Target Milestone: --- → Thunderbird 18.0
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → kent
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 27•12 years ago
|
||
> I'd like to keep either this or a related bug open
> to ... make GetOfflineFileStream behave better.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 28•12 years ago
|
||
Thanks for catching that I forgot that, Ben! But let's do it in a new Bug 791893.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 29•12 years ago
|
||
Not significant enough for tracking...
Comment 30•12 years ago
|
||
Comment on attachment 661854 [details] [diff] [review] Patch as landed (added MPL header to test), r=irving [Triage Comment] ...I will take it onto aurora and beta though, as I think this is a low risk enough simple null-check addition.
Attachment #661854 -
Flags: approval-comm-beta+
Attachment #661854 -
Flags: approval-comm-aurora+
Comment 31•12 years ago
|
||
Checked in: http://hg.mozilla.org/releases/comm-aurora/rev/4ad0797d11d0 https://hg.mozilla.org/releases/comm-beta/rev/80fd034a8f21
status-thunderbird16:
--- → fixed
status-thunderbird17:
--- → fixed
Reporter | ||
Comment 32•12 years ago
|
||
v.fixed no tb16 crashes in https://crash-stats.mozilla.com/report/list?product=Thunderbird&query_search=signature&query_type=exact&query=NS_ReadLine%26lt%3Bchar%2C%20nsIInputStream%2C%20nsCAutoString%26gt%3B&reason_type=contains&date=10%2F18%2F2012%2012%3A15%3A22&range_value=4&range_unit=weeks&hang_type=any&process_type=all&do_query=1&admin=1&signature=NS_ReadLine%3Cchar%2C%20nsIInputStream%2C%20nsCAutoString%3E and none on trunk
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•