236215 - certain selection and junk mail maneuvers in thread pane cause crash in nsTreeRange::Count after timeout

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Tuukka Tolvanen (sp3000)]

User-Agent:       
Build Identifier: 

tested: thunderbird 0.5, trunk cvs
        seamonkey 2004-02-26-09-trunk nightly

Reproducible: Always
Steps to Reproduce:
1. Have junk mail from more than one past day in junk folder
2. Set Tools > junk mail controls > automatically delete messages older than n
   days to some value of n within your junk mail
3. View Junk folder unthreaded (View > Messages > Threaded unset), in order
   received (View > Sort by > order received)
4. in thread pane:
    1. select top message
    2. mark as junk
    3. shift-pagedown
    4. shift-downarrow
    5. mark as junk
    6. uparrow
    7. downarrow
    8. shift-uparrow
    9. shift-pagedown
   10. shift-downarrow
   11. mark as junk
5. wait a few minutes for the crash (guessing it's the junk mail purge timer,
   every five minutes apparently)
Actual Results:  
crash

Expected Results:  
no crash

Haven't gotten the STR any simpler than that so far, sorry... Tested also with
new profile, but using an old Junk mail folder. Interestingly, doing the same
steps in Inbox on an account with the "when I manually mark ... send to junk
folder" pref on caused a hang the one time I tested that.

Comment 1

[Tuukka Tolvanen (sp3000)]

Attachment: bt full

Comment 2

[Tuukka Tolvanen (sp3000)]

Attachment: printf dumpage and bt full

Crashed in nsTreeSelection::AdjustSelection this time. I instrumented
nsTreeRange with a few printf's, and here's what seems to happen during the
STR:

    1. select top message
 @ nsTreeRange::nsTreeRange : mMin == mMax == aSingleval == 0
 @ nsTreeRange::Count : mMin == 0, mMax == 0, mNext == (nil)
    2. mark as junk
    3. shift-pagedown
 @ nsTreeRange::nsTreeRange : mMin == 0, mMax == 6
 @ nsTreeRange::Count : mMin == 0, mMax == 6, mNext == (nil)
    4. shift-downarrow
 @ nsTreeRange::Count : mMin == 0, mMax == 7, mNext == (nil)
    5. mark as junk
    6. uparrow
 @ nsTreeRange::Count : mMin == 6, mMax == 6, mNext == (nil)
    7. downarrow
 @ nsTreeRange::nsTreeRange : mMin == mMax == aSingleval == 7
 @ nsTreeRange::Count : mMin == 7, mMax == 7, mNext == (nil)
    8. shift-uparrow
 @ nsTreeRange::nsTreeRange : mMin == 6, mMax == 7
 @ nsTreeRange::Count : mMin == 6, mMax == 7, mNext == (nil)
    9. shift-pagedown
 @ nsTreeRange::nsTreeRange : mMin == 7, mMax == 7
 @ nsTreeRange::nsTreeRange : mMin == 8, mMax == 12
 @ nsTreeRange::Count : mMin == 6, mMax == 5, mNext == 0x8a5f988
 @ nsTreeRange::Count : mMin == 7, mMax == 7, mNext == 0x8ec0220
 @ nsTreeRange::Count : mMin == 8, mMax == 12, mNext == (nil)
   10. shift-downarrow
 @ nsTreeRange::Count : mMin == 6, mMax == 5, mNext == 0x8a5f988
 @ nsTreeRange::Count : mMin == 7, mMax == 7, mNext == 0x8ec0220
 @ nsTreeRange::Count : mMin == 8, mMax == 13, mNext == (nil)
   11. mark as junk

...so apparently shift-uparrow, shift-pagedown in the thread pane yields a
three-piece nsTreeRange chain where the first one looks bogus.

Comment 3

[[e-mail]]

nsTreeRange doesn't know how to clear a subrange that begins or ends at the
start or end of the range - but it's ok if the subrange includes both end
points, or lies partially outside the range.

Comment 4

[[e-mail]]

Attachment: Proposed patch

This stops the first bogus range from being created, but there are still two
ranges left, which is somewhat odd...

Comment 5

[[e-mail]]

Comment on attachment 143119 [details] [diff] [review]
Proposed patch

This fix prevents RemoveRange from creating zero-length ranges, but tree.xml
should still be fixed to use the selection correctly.

Comment 6

[Tuukka Tolvanen (sp3000)]

Yep, that does seem to fix the crash for me, too.

Comment 7

[Scott MacGregor]

Jan, have you had a chance to review this patch yet for this crash fix?

Comment 8

[Jan Varga [:janv]]

Comment on attachment 143119 [details] [diff] [review]
Proposed patch

This patch is actually a rewrite of the RemoveRange method. Both
implementations are quite hard to understand, so I will blindly believe that
it's been tested heavily.
r=varga

Comment 9

[[e-mail]]

Comment on attachment 143119 [details] [diff] [review]
Proposed patch

Well I've been running with the patch for weeks, and that includes testing tree
selections for bug 97434.

Comment 10

[Scott MacGregor]

Comment on attachment 143119 [details] [diff] [review]
Proposed patch

If we are worried about it, now's a good time to check it in and get testing
coverage on it :)

Comment 11

[[e-mail]]

Fix checked in.

Comment 9 was in error, in fact my attachment to bug 97434 actually reduces the
chance of hitting this bug.

Comment 12

[[e-mail]]

Really marking fixed.

Comment 13

[sairuh (rarely reading bugmail)]

vrfy'd fixed with 2004112311-0.9 on linux fc2, following the steps in comment 0.