30894 - CRASH: reply to a mail message, close the compose window before the reply is filled in

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P3)

Description

[David :Bienvenu]

I replied to a long message, and realized it was going to take too long to 
quote, so I closed the compose window. I crashed in the editor code in 
nsInterfaceState::SelectionIsCollapsed() with mEditor having been deleted. I'll 
try to get a full stack trace in a minute.

Comment 1

[David :Bienvenu]

Looks like the editor has set some timer, but hasn't cancelled the timer when 
the editor got deleted.

nsQueryInterface::operator()(const nsID & {...}, void * * 0x0012fc70) line 32 + 
23 bytes
nsCOMPtr<nsIEditor>::assign_from_helper(const nsCOMPtr_helper & {...}, const 
nsID & {...}) line 795 + 18 bytes
nsCOMPtr<nsIEditor>::nsCOMPtr<nsIEditor>(const nsQueryInterface & {...}) line 
508
nsInterfaceState::SelectionIsCollapsed() line 320
nsInterfaceState::TimerCallback() line 248 + 11 bytes
nsInterfaceState::Notify(nsInterfaceState * const 0x04976e7c, nsITimer * 
0x04b0ae30) line 528
nsTimer::Fire() line 200
nsTimerManager::FireNextReadyTimer(nsTimerManager * const 0x018ed880, unsigned 
int 0) line 117
nsAppShell::Run(nsAppShell * const 0x0143ca40) line 116
nsAppShellService::Run(nsAppShellService * const 0x0143a370) line 393
main1(int 2, char * * 0x010a4cd0, nsISplashScreen * 0x00000000) line 769 + 32 
bytes

Comment 2

[David :Bienvenu]

I'm nominating this as a beta bug, just because it's an easily reproducible 
crash.

Comment 3

[buster]

simon...oh, simon...

Comment 4

[Simon Fraser [no longer active]]

Hmm. nsInterfaceState should have cancelled that timer; it does it in the dtor. 
Investigating.

Comment 5

[Simon Fraser [no longer active]]

Ah, I suck. The dtor never gets called in that situation because a ref is held by 
the timer code.

The fix is to move the following lines:

  // cancel any outstanding udpate timer
  if (mUpdateTimer)
    mUpdateTimer->Cancel();

from nsInterfaceState::~nsInterfaceState() to 
nsInterfaceState::NotifyDocumentWillBeDestroyed().

(sorry, no diff, I have tons of other changes in that file).

Comment 6

[buster]

r=buster

Comment 7

[David :Bienvenu]

d'oh! thanks for finding and fixing this so quickly, Simon.

Comment 8

[Phil Peterson]

PDT-

Comment 9

[David :Bienvenu]

I'm a little surprised this is PDT-, it looks like a very safe fix for a crasher 
bug that actual users are going to run into. I'm not trying to be a pain, but 
I'll try removing the PDT- once.

Comment 10

[buster]

I'd like to throw in my vote for PDT+ on this one as well. It's a very safe fix, 
and stability will be the number 1 criteria on which we are judged in my 
opinion.  The chances of a regression from this fix are very slim.

Comment 11

[leger]

Putting on PDT+ radar for beta1.  Must fix by 03/09.

Comment 12

[Simon Fraser [no longer active]]

Fix checked in.

Comment 13

[laurel]

OK using 2000-03-10-09m15 commercial build on NT 4.0.
OK with both plain text and compose windows.

Comment 14

[laurel]

OK using 2000-03-10-13m15 commercial build linux rh6.0

Comment 15

[laurel]

OK using 2000-03-10-14m15 commercial build mac OS 9.0