25269 - New profile, Acct Wiz crashes selecting Cancel or Finish

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect, P3)

Description

[Ninoschka Baca]

Build 2000012609M14: NT4

Overview: In a new profile when Mail starts the Account Wizard opens. Select the 
Cancel or Finish button and it crashes. This only happens if Mail is the only 
component open. If the Browser is open at the time then it does not crash.

Steps to reproduce:
1. Start using "mozilla -mail"
2. The "User Select Profiles" window appears, select the "Manage Profiles" 
button
3. Select the "New Profile" button, select the "Next" button and enter the name 
of the new profile and select the "Finish" button.
4. From the list of profiles select the new profile and select the "Start 
Mozilla" button.
5. Mail opens, then the Account Wizard opens.
6. Select the "Cancel" button or the "Finish button.

Actual Results: A crash occurs.
Expected Results: 
a. Selecting the Cancel button should close the Account Wizard dialog and return 
focus to the Mail window. 
b. Selecting the Finish button should do nothing because it appears gray and 
should be disabled.

Additional Information:
- Incident Numbers for NT4: http://cyclone/main/talkback.cfm
* 4526158: Crash after selecting the Cancel button.
* 4525610: Crash after selecting the Finish button.

- Linux 6.0: After step #4 the Mail window opens and then crashes. I never see 
the Account Wizard.
* 4528876: Crash after launching new profile with "-mail".

- NT and Linux: If the Browser is open then the crash does not occur.

Comment 1

[laurel]

Possibly related to bug #24396?

Comment 2

[Alec Flett]

the last stack trace is a dupe of bug 25181.

Both of these bugs are JS/XPConnect I think, judging by the stack traces.
reassigning to brendan and adding jband to CC.. I have a feeling that all of
these are all linked to the same general problem.

Comment 3

[Mike McCabe]

These are probably *not* JS/XPConnect, as the stack trace never gests as far as
the JS engine.  For 25181, it looks like a timer may have been set (possibly by
javascript) then called after its target window has disappeared... but the crash
occurs before the DOM timer-handling code calls the JS engine.

I've tentatively reassigned 25181 to Vidur, as it looks like a timer issue. 
This bug could be something similar.

Comment 4

[Mike McCabe]

Any connection with the checkin of 22979?

Comment 5

[Alec Flett]

yes, the timer is actually set by my javascript. the window opens on a timer (by
design) maybe the bug is something like "crash when the FIRST window opens
another window on a timer"?

Comment 6

[Alec Flett]

MIGHT be related. it's a scary checkin to try to backout and test..maybe someone
from QA can test the builds immediately before and after the 1-21-00-afternoon
build

Comment 7

[Brendan Eich [:brendan]]

Someone needs to debug a little more.  McCabe is right: the two talkback stacks 
show nsGlobalWindowImpl::RunTimeout calling JS_GetFunctionObject, which then 
crashes.  Problem is, the current top-of-trunk nsGlobalWindow.cpp has no such 
JS_GetFunctionObject call anywhere in it, and never has, AFAIK.  So either the 
talkback symbols lie (surprise), and the top frame is not JS_GetFunctionObject; 
or some bad change that I haven't dug out of CVS yet was made, and then fixed.

If talkback symbols are screwed, perhaps the crash is in JS_GetStringChars.  If 
that's true, the argument (timeout->expr) must be bad.  Who can check on either 
talkback or the faulting address (i.e., is timeout->expr null, if indeed the 
crash is in JS_GetStringChars)?

Alec, can you reproduce this under a debugger?  If so, I'd like to reassign this 
to you.  It shouldn't be in my list for long, as I'm on sabbatical after this 
week.

/be

Comment 8

[Brendan Eich [:brendan]]

Timers are implicated.  Alecf and I found two nsTimeoutImpls hanging off the 
nsGlobalWindowImpl, linked together but not consistent (some members of each 
struct were garbage; expr and funobj were both non-null, which is wrong; argc 
was garbage on one; public_id was 0 on both, etc.).

Reassigning to [e-mail] -- Michael, this bug may not be in the 
new timer code, but it seems correlated.  Can you have a look, consult with 
vidur, and see where this bug should go for fixing?  Thanks,

/be

Comment 9

[Michael Lowe]

Accepting bug

Comment 10

[Michael Lowe]

I've had a look at this bug and I'm almost certain it's a re-entrancy problem 
with:

PRBool GlobalWindowImpl::RunTimeout(nsTimeoutImpl *aTimeout)

I'll post more details later, but I'm 99% sure it's not a problem with my new 
timer code.

Comment 11

[Michael Lowe]

Here is the general gist of the problem:

1. widgetglue.js calls SetTimeout and a nsITimer is created
2. sidebaroverlay.js calls SetTimeout and a nsITimer is created
3. nsITimer for sidebaroverlay.js fires and calls GlobalWindowImpl::RunTimeout
4. GlobalWindowImpl::RunTimeout looks through its list of timers and decides to 
fire the timer for widgetglue.js first
5. timer routine in widgetglue.js opens a new dialog and enters a modal event 
loop (GlobalWindowImpl::RunTimeout is still on the call stack from previous 
step)
6. The dialog modal loop fires the nsITimer for widgetglue.js and calls 
GlobalWindowImpl::RunTimeout (again).   Bang!   Multiple re-entancy problems!!!

If I put a simple lock around GlobalWindowImpl::RunTimeout in 
nsGlobalWindow_RunTimeout:

void
nsGlobalWindow_RunTimeout(nsITimer *aTimer, void *aClosure)
{
  nsTimeoutImpl *timeout = (nsTimeoutImpl *)aClosure;

  static int lock = 0;
  if (lock == 1) return;
  lock = 1;

  if (timeout->window->RunTimeout(timeout)) {
    // Drop the ref_count since the timer won't be holding on to the
    // timeout struct anymore
    timeout->window->DropTimeout(timeout);
  }

  lock = 0;
}

the problem goes away.

Reassign to vidur to decide what the real fix should be here.

Another issue I noticed is that GlobalWindowImpl::RunTimeout can fire multiple 
Javascript DOM timers off a single nsITimer event.   To provide a responsive 
user-interface this is probably not the best approach to take.

Comment 12

[leger]

Adding "crash" keyword to all known open crasher bugs.

Comment 13

[vidur (gone)]

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

Comment 14

[Ninoschka Baca]

Build 2000012812M14: NT4

Another scenario: When I use "-mail" and create a new profile, the first Account 
Wizard dialog appears twice.

Steps to reproduce:
1. Run "mozilla -mail"
2. Mail opens and the first Account Wizard dialog appears showing a choice to 
choose Mail or News
3. Don't enter any information, just move the Account Wizard dialog to a 
different location.

Actual Results: Notice that the same Account Wizard dialog appears a second 
time. I can advance through the Wizard dialogs, select Finish and I can see the 
account listed in the Folders pane. To remove the second Account Wizard dialog I 
select the close box "x" and it crashes.


Additional Information:
- If the Browser window is open first, then Mail is opened and only one Account 
Wizard dialog appears.
- Linux: I don't see two Account Wizard dialogs but it crashes after selecting 
the "Next" button after it displays the Account Name (the dialog before the last 
dialog).

Comment 15

[Michael Lowe]

This is another symptom of the multiple-reentrancy problem.

Comment 16

[laurel]

OK, can we get a target milestone set on this, like m14?  It's a definite
hinderance.

Comment 17

[Phil Peterson]

Nominate as beta1 stopper

Comment 18

[laurel]

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

Comment 19

[leger]

Putting on on PDT+ radar for beta1.

Comment 20

[Alec Flett]

thanks for looking at this vidur

Comment 21

[Phil Peterson]

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

Comment 22

[Ninoschka Baca]

Build 2000-02-11-11M14: Linux
A slightly different scenario where a crash occurs when the Browser is closed.

1. Delete the registry file and all accounts. 
2. Run "./mozilla -mail"
3. The Create New Profile dialog appears so enter a profile name and select the 
Finish button.
4. Mail opens immediately followed by the Account Wizard
5. Select to create a mail account
6. Progress through all the Wizard dialogs until the Congratulations dialog 
appears.
7. At the Congratulations dialog select the Finish button 

Actual Results: A crash occurs. 
Note: This only happens if the Browser is not open. If the browser is open then 
the crash does not occur. Also, this crash occurs on the last dialog when 
selecting "Finish". 
Expected Results: No crash should occur.

Comment 23

[vidur (gone)]

Michael Lowe's analysis was correct, though the fix was a bit more complicated. 
Rather than prevent reentering of RunTimeout (and run the risk of dropping 
timeouts), the code is now reentrant. Fix checked in on 2/14/2000.

Comment 24

[vidur (gone)]

Oops, forgot to change the status.

Comment 25

[Ninoschka Baca]

Build 2000-02-29-08M15, Commercial: NT4, Linux 6.0
Build 2000-02-29-08M15, Mozilla: Mac 8.5.1
Verified Fixed. 

1. The Cancel and Finish buttons on the Account Wizard's first dialog no longer 
crashes. 
2. The problem with multiple windows appearing is not there.
3. Selecting the Finish button on the Congratulations screen also does not 
produce a crash.