45184 - Crash when IMAP filter fires to move message to local or other account. [@ rdf_HashWideString]

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P2)

Description

[laurel]

Using july 11 commercial build NT 4.0 and linux rh6.0
Haven't tried mac yet, will update comments

Found while verifying bug #17174

When IMAP fires a filter to move an incoming message to a local folder or
another IMAP account folder, a crash occurs either upon Get Msg/when it fires or
when opening the folder where the message was placed.

This happens with both migrated filters and filters created with new seamonkey
ui (on a migrated profile, haven't yet verified on new profile).

Haven't tried all variations of move to other accounts yet...

Steps to reproduce:

1.  Go to mail window, login to IMAP account.
2.  Edit|Message Filters, select the IMAP account in the filter scope dropdown.
3.  Set up a simple message filter like "subject, contains, foo" with an action
to move to a folder in the Local Folders hierarchy.
    Confirm OK out of filters dialogs.
4.  Send your IMAP account a message which matches the filter criteria. Get
messages on that IMAP account.
    Result: Crash may occur here. If not proceed.
5.  Open the destination local folder.
    Result: Crash will occur here if it didn't in step 4.

Note:  Every once in awhile I don't see a crash, I believe when initially
logging into an account where filter fires.  90% of the instances tried have
crashed for me.

Repeat scenario, but set filter to filter to another IMAP account folder (I
tried where I was already logged into both IMAP accounts, haven't tried
variations of authentication yet.) Same results.

Comment 1

[laurel]

added crash, nsbeta2,nsbeta3 keywords.
Don't know how common it will be to have filters act to move messages to the
Local Folders hierarchy or another account, but it's currently a crash
situation.

Will post talkback incident when I've retrieved it.

Comment 2

[laurel]

Attachment: talkback incident 13985076

Comment 3

[Alec Flett]

I reproduced the crash, looks like the biff landing might have broke this... 

Comment 4

[David :Bienvenu]

Attachment: patch for part of the problem

Comment 5

[David :Bienvenu]

*** Bug 45179 has been marked as a duplicate of this bug. ***

Comment 6

[Jon Granrose]

adding self to cc.

45179 is a reproducible crash when doing "Message|Mark|All Read" in news that
has the same stack trace as this bug.

Comment 7

[Jay Patel [:jay]]

Adding topcrash keyword, this made it to the top5 talkback crash list today.

Comment 8

[Jay Patel [:jay]]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 9

[David :Bienvenu]

I think I see the problem - don't we need the following line at the end of
nsMsgMessageDataSource::createStatusNodeFromFlag():

  NS_IF_ADDREF(*node);

- David

Comment 10

[Alec Flett]

good lord! we do. Nice catch..
r=alecf if you want to check in.

Comment 11

[David :Bienvenu]

OK, I'll take this, and check in the fixes. and take out those
NS_ENSURE_TRUE(rv, rv), which is bogus, right, because rv == 0.

Comment 12

[David :Bienvenu]

that change seems to fix the problem.

Comment 13

[scottputterman]

good catch. Sorry about that.  I missed that in the code review.

Comment 14

[scottputterman]

*** Bug 45273 has been marked as a duplicate of this bug. ***

Comment 15

[David :Bienvenu]

*** Bug 45277 has been marked as a duplicate of this bug. ***

Comment 16

[David :Bienvenu]

*** Bug 45278 has been marked as a duplicate of this bug. ***

Comment 17

[Alec Flett]

oops, I think that NS_ENSURE_TRUE should have been NS_ENSURE_SUCCESS(rv,rv)

Comment 18

[David :Bienvenu]

yes, that what I changed it to in my fix.

Comment 19

[David :Bienvenu]

fix checked in.

Comment 20

[Alec Flett]

*** Bug 45362 has been marked as a duplicate of this bug. ***

Comment 21

[laurel]

OK using july 13 commercial build, linux rh6.0, NT 4.0 and mac OS 9.0