593007 - Stack overflow on corrupted newsgroup Crash [@ arena_malloc_small ] looping through nsMsgQuickSearchDBView::ListIdsInThreadOrder

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Bruno 'Aqualon' Escherl]

The original bug wasn't cloneable, so creating it manually.

There are still a lot of crashes with that signature. Can't say the exact number, since not all crashes with signature arena_malloc_small are related to nsMsgQuickSearchDBView::ListIdsInThreadOrder.

Looking at last weeks crashstats affected versions are SeaMonkey 2.0.6, Thunderbird 3.0.4, 3.0.6, 3.0.7, 3.1.1 and 3.1.2. There are no reported crashes for trunk builds or any pre version.

Comment 1

[Ludovic Hirlimann [:Usul]]

Would be nice to figure out some STRS.

Comment 2

[David :Bienvenu]

Thx to Günter, I have a .msf file and news.mozilla.org.rc file that enable me to recreate the crash.

Comment 3

[David :Bienvenu]

Attachment: proposed fix

This fixes the crash. I'm going to try to do a bit more digging into what exactly is wrong with the threading info in the .msf file, and how it might have gotten that way.

Comment 4

[David :Bienvenu]

Attachment: fix one cause of corruption

This should fix one (the?) cause of the corruption - adding a hdr that already exists to a thread confuses the threading structure. We should never do that. One reproducible way of doing this is to repair a newsgroup folder, and get multiple downloads going on the newsgroup.

Comment 5

[David :Bienvenu]

Comment on attachment 485771 [details] [diff] [review]
proposed fix

This basically extends the previous sanity check to include skipped levels. I'm going to try to write a unit test for this, but creating a horked db is probably going to prove challenging.

Comment 6

[Ludovic Hirlimann [:Usul]]

(In reply to comment #5)
> Comment on attachment 485771 [details] [diff] [review]
> proposed fix
> 
> This basically extends the previous sanity check to include skipped levels. I'm
> going to try to write a unit test for this, but creating a horked db is
> probably going to prove challenging.

Can't you reuse the one you analyzed ?

Comment 7

[David :Bienvenu]

It's 3MB, a few orders of magnitude bigger than the minimal test case.

Comment 8

[David :Bienvenu]

Comment on attachment 485895 [details] [diff] [review]
fix one cause of corruption

We shouldn't allow adding a header that already exists...

Comment 9

[David :Bienvenu]

Attachment: wip on unit test

saving what I had so far for the unit test. I haven't been able to reproduce the exact structure of the thread that was causing the crash fixed in the view code, however.

Comment 11

[David :Bienvenu]

fixed for 3.1.7 - http://hg.mozilla.org/releases/comm-1.9.2/rev/dc08836c0a76