101845 - Crash if exit while File from Search ongoing.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[laurel]

Using sep26 branch build
Seen on Win98 & Mac OS X  so assuming all platforms

If a File from search messages UI results pane is ongoing and you exit, a crash
occurs. (Note: was not able to crash if doing the same kind of thing from main
mail window.)

Not something that may be encountered a lot, but we shouldn't crash.

Steps:
1.  Search a large newsgroup (I used n.p.m.mail-news) to yield a lot of matches
(I  had 1400-2000).
2.  Select all search results (current Select All doesn't work in search, use
Shift+_Select).
3.  Click File button in search ui and select a mail folder destination. (I
chose a folder in Local Folders.)
4.  File/copy operation begins... while still in progress, File|Quit.

Result: crash occurs. will attach talkback report.

Comment 1

[laurel]

Talkback incident 35926967:
Stack Trace
nsNNTPProtocol::DisplayArticle
[d:\builds\seamonkey\mozilla\mailnews\news\src\nsNNTPProtocol.cpp, line 2518]
nsNNTPProtocol::ReadArticle
[d:\builds\seamonkey\mozilla\mailnews\news\src\nsNNTPProtocol.cpp, line 2560]
nsNNTPProtocol::ProcessProtocolState
[d:\builds\seamonkey\mozilla\mailnews\news\src\nsNNTPProtocol.cpp, line 5127]
nsMsgProtocol::OnDataAvailable
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgProtocol.cpp, line 246]
nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp, line 188]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524]
nsEventQueueImpl::ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\nsEventQueue.cpp, line 375] 

Comment 2

[laurel]

Tried briefly to reproduce using large File from mail message Search results and
did not crash. Maybe only a newsgroup search problem.

Comment 3

[Navin Gupta]

Are you crashing on trunk as well ?. I am not seeing any crash on trunk.

Comment 4

[laurel]

I didn't try on the trunk yet. Will do so when I get a chance.

Comment 5

[laurel]

I tried with both today's (sep27) branch and trunk builds on win98.  Was able to
crash most of the time on both branch and trunk, but did make it through once on
the trunk without crashing. Sent another talkback in, but can't retrieve it yet.

Comment 6

[Navin Gupta]

ok, I will try again, but I am having hard time reproducing it in VC++ debugger.

Comment 7

[laurel]

adding keywords

Comment 8

[Jaime Rodriguez, Jr.]

Is this really a nsbeta1+, P2? If yes, then we need to try and targeted to a
mielstone M1.0 or earlier to make the beta.

Comment 9

[Navin Gupta]

Attachment: proposed fix

The fix is to pass a valid msgWindow, which was not happening here because when

we file messages from search we don't allow undo. Use mCopyState->m_msgWindow
so that on File |Quit we are able to cancel the running news url.

Comment 10

[Navin Gupta]

crash doesn't happen for imap, only for local. david, you can also review because
fix is in local folders, but I have asked seth too. 

Comment 11

[(not reading, please use [e-mail] instead)]

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

so the old code uses the msgWindow from the undo txn object, and the new way
used the msg window from the copy state.

which scenarios have you tested?

I'm worried about other scenarios where the this switch will cause problems,
due to us not having a msg window where we did before.

Comment 12

[Navin Gupta]

if you look at nsLocalMailFolder::CopyMessages it is same msgWindow for 
both copyState and undoObject. 

Since I have made change to EndCopy in local folder I have tried copying & 
moving msgs from local,imap, news to local and undoing them and it works fine. 
Also Search: filing from news, imap, local to local work fine.

Comment 13

[(not reading, please use [e-mail] instead)]

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

r=sspitzer

Comment 14

[David :Bienvenu]

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

sr=bienvenu

Comment 15

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.9

Comment 16

[Navin Gupta]

fixed

Comment 17

[laurel]

OK using feb25 commercial trunk build: win98, mac OS 10.1, linux rh6.2