493186 - Crash when switching newsgroup [@ nsMsgThreadedDBView::MoveThreadAt]

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Peter Weilbacher]

With recent SM from comm-central (self built on Linux amd64) I sometimes see crashes in nsMsgThreadedDBView::MoveThreadAt, even with the fix from bug 492475 applied. I build with --disable-debug --enable-debugger-info-modules=yes and run under gdb to find these crashes.

Today gdb finally gave me this:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f4575c24750 (LWP 14370)]
_______________________________________________________________________________
Error while running hook_stop:
Value can't be converted to integer.
0x00007f4567f23129 in nsMsgThreadedDBView::MoveThreadAt (this=0x7f454a9c6800, threadIndex=0x0) at /home/mozilla/comm-central/mailnews/base/src/nsMsgThreadedDBView.cpp:814
814	  m_flags[newIndex] = saveFlags;
gdb> print newIndex
$1 = 0xffffffff
gdb> print saveFlags
$2 = 0x48040020
gdb> bt
#0  0x00007f4567f23129 in nsMsgThreadedDBView::MoveThreadAt (this=0x7f454a9c6800, threadIndex=0x0) at /home/mozilla/comm-central/mailnews/base/src/nsMsgThreadedDBView.cpp:814
#1  0x00007f4567f2355e in nsMsgThreadedDBView::OnNewHeader (this=0x7f454a9c6800, newHdr=0x7f45435fd4c0, aParentKey=0x81b7, ensureListed=0x0) at /home/mozilla/comm-central/mailnews/base/src/nsMsgThreadedDBView.cpp:678
#2  0x00007f4567fe8590 in nsMsgDatabase::NotifyHdrAddedAll (this=<value optimized out>, aHdrAdded=0x7f45435fd4c0, aParentKey=0x81b6, aFlags=0x11, aInstigator=0x0) at /home/mozilla/comm-central/mailnews/db/msgdb/src/nsMsgDatabase.cpp:688
#3  0x00007f4567feb5b7 in nsMsgDatabase::AddNewHdrToDB (this=0x7f4543f3f340, newHdr=0x7f45435fd4c0, notify=0x1) at /home/mozilla/comm-central/mailnews/db/msgdb/src/nsMsgDatabase.cpp:3020
#4  0x00007f45680b6b8c in nsNNTPNewsgroupList::CallFilters (this=0x7f453b1835c0) at /home/mozilla/comm-central/mailnews/news/src/nsNNTPNewsgroupList.cpp:1157
#5  0x00007f45680bc778 in nsNNTPProtocol::ProcessXover (this=0x7f4539030800) at /home/mozilla/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:3557
#6  0x00007f45680c9095 in nsNNTPProtocol::ProcessProtocolState (this=0x7f4539030800, url=0x7f4543ad08a8, inputStream=0x7f45445dfa10, sourceOffset=<value optimized out>, length=0x101) at /home/mozilla/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:5136
#7  0x00007f4567eb985d in nsMsgProtocol::OnDataAvailable (this=0x7f4539030818, request=<value optimized out>, ctxt=<value optimized out>, inStr=0x7f45445dfa10, sourceOffset=0x9854, count=0x101) at /home/mozilla/comm-central/mailnews/base/util/nsMsgProtocol.cpp:350
#8  0x00007f4567412c5e in nsInputStreamPump::OnStateTransfer (this=0x7f454782fc80) at /home/mozilla/comm-central/mozilla/netwerk/base/src/nsInputStreamPump.cpp:508
#9  0x00007f4567412de8 in nsInputStreamPump::OnInputStreamReady (this=0x7f454782fc80, stream=<value optimized out>) at /home/mozilla/comm-central/mozilla/netwerk/base/src/nsInputStreamPump.cpp:398
#10 0x00007f4574e79837 in nsInputStreamReadyEvent::Run (this=<value optimized out>) at /home/mozilla/comm-central/mozilla/xpcom/io/nsStreamUtils.cpp:111
#11 0x00007f4574e94e1b in nsThread::ProcessNextEvent (this=0x7f456d62e160, mayWait=0x1, result=0x7fff7dc5e5ec) at /home/mozilla/comm-central/mozilla/xpcom/threads/nsThread.cpp:510
#12 0x00007f4574e52af9 in NS_ProcessNextEvent_P (thread=0x7f454a9c6850, mayWait=0x1) at nsThreadUtils.cpp:227
#13 0x00007f4565419e3d in nsBaseAppShell::Run (this=0x7f4568a90940) at /home/mozilla/comm-central/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:170
#14 0x00007f456488fa76 in nsAppStartup::Run (this=0x7f4568aa06c0) at /home/mozilla/comm-central/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:193
#15 0x00007f4575326aab in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>) at /home/mozilla/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:3298
#16 0x0000000000401b19 in main (argc=0x1, argv=0x7fff7dc5edb8) at /home/mozilla/comm-central/suite/app/nsSuiteApp.cpp:103

I guess this means that the FindHdr(threadHdr) fallback doesn't always work.

Comment 1

[Peter Weilbacher]

I'm continuing to get these crashes about once per day.

FindHdr() is probably nsMsgDBView::FindHdr() in that case, and that has
   if (viewIndex == nsMsgViewIndex_None)
     return viewIndex;
so it can very well return 0xffffffff. (Setting newIndex=0 as another fallback in MoveThreadAt() probably fixes the SIGSEGV crashes, but I have no clue what this code does, so I won't even try that to not mess with my threaded views...)

Comment 2

[Peter Weilbacher]

Attachment: test fix

To move this forward, I provide this test fix. Simply resetting the index to 0 at least fixed my crashes. I have no idea what repercussions this can have, but I have been running with this fix for 3 days now and haven't seen a single crash. I saw some instances of the debug output on the console but never noticed any problems at the same time...

Comment 3

[David :Bienvenu]

Comment on attachment 385340 [details] [diff] [review]
test fix

thx for catching this in the debugger - instead of 0xffffffff, you should use nsMsgViewIndex_None, and I think instead of the #ifdef DEBUG, you should use someting like NS_WARNING("new index -1 in MoveThreadAt")

I'd still like to know how newIndex ends up at -1, but it's better not to crash.

Comment 4

[Peter Weilbacher]

Attachment: fix

Improved patch as requested.

Perhaps one could also change nsMsgDBView::FindHdr instead of this. Is 
   if (viewIndex == nsMsgViewIndex_None)
       return viewIndex;
as checked in by bug 482195 maybe a typo that should have read
       return 0;
? I'm going to try that for my next build.

Comment 5

[David :Bienvenu]

(In reply to comment #4)

> 
> Perhaps one could also change nsMsgDBView::FindHdr instead of this. Is 
>    if (viewIndex == nsMsgViewIndex_None)
>        return viewIndex;
> as checked in by bug 482195 maybe a typo that should have read
>        return 0;
> ? I'm going to try that for my next build.

No, 0 would mean we found it (0 is a valid view index). So it's the callers that have to deal with this, not FindHdr.

Comment 6

[David :Bienvenu]

Comment on attachment 385752 [details] [diff] [review]
fix

thx, fix the first brace *not* to be K&R, and r/sr=me

Comment 7

[Peter Weilbacher]

Pushed, with brace fix:
http://hg.mozilla.org/comm-central/rev/c20a20417bbb

Comment 8

[Wayne Mery (:wsmwk)]

(crash=critical)

Comment 9

[Wayne Mery (:wsmwk)]

unable to verify via crash-stats .... after bug 492475 was fixed, this sig doesn't exist on crash-stats