200006 - cannot dismiss alert when accessing a bogus url via open web location (target=current window or new tab, new window works fine)

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2003.03.31.03 comm bits on mac 10.2.4. not a problem on win2k or
linux. this is a recent regression, as it works fine in the 2003.03.25.03 build.

1. bring up the Open Web Location dialog (cmd+shift+L).
2. select "New Navigator Tab" from the droplist.
3. enter a bogus url in the location textfield (eg, foo.foo.foo).
4. click the Open button.
5. you'll get an alert (sheet) saying that the url is invalid --click OK to
dismiss the alert.

results: the alert sheet won't go away. with the exception of the apple and
application menu, the entire menubar is disabled. you can only quit the app.

Comment 1

[sairuh (rarely reading bugmail)]

this regressed btwn the 2003.03.25.03 (works fine) and 2003.03.26.15 (broken)
builds.

Comment 2

[Paul Wyskoczka]

Nav triage team: nsbeta1+/adt2

Comment 3

[jag (Peter Annema)]

Haven't we seen this before? I guess I'll go find the bug and see how they fixed
it then (though I seem to recall that one being cross platform).

Comment 4

[jag (Peter Annema)]

See (related or same?) bug 144710

Comment 5

[sairuh (rarely reading bugmail)]

odd how it was working for a while, then came back.

Comment 6

[Chris Casciano]

Going back with the builds i have on hand I see this *is* a problem on for me on
1.3,  was not on 2003031403 on the first try (but the focus was lost), 2nd try
(relaunched app) i wasn't able to dismiss the dialog again... 

Perhaps this isn't a regression, but just intermittent (and a dupe of bug
144710) - Se, in 2003 03 25 03 did it work ok multiple times in a row?

Comment 7

[jag (Peter Annema)]

I pulled a tree from 2003-03-25 00:00:00 and saw this problem.

*** This bug has been marked as a duplicate of 144710 ***

Comment 8

[jag (Peter Annema)]

Not quite a dupe after all, just very similar symptoms.

Comment 9

[jag (Peter Annema)]

So here's where I'm at: when you call window.setTimeout, we add a timer to a
sorted queue. For this specific sheet case, that timer never gets processed,
even though its time has come up and it should've been.

Comment 10

[jag (Peter Annema)]

*mumble* modal event loops *mumble*

Comment 11

[Simon Fraser [no longer active]]

Attachment: Sampler data showing stacks for thread deadlock

Comment 12

[Simon Fraser [no longer active]]

The data in attachment 120977 [details] show what's going on here. Essentially, we're
deadlocked on the event queue monitor.

The main thread is in a nested event loop for the 'host not found' dialog. On
the stack, we're just about to leave nsWindowWatcher::OpenWindowJS(), which was
called for the Open Location dialog. That has a EventQueueAutoPopper on the
stack, which is being destroyed, and in doing so calling
nsEventQueueServiceImpl::PopThreadEventQueue. That method enters the
mEventQMonitor of the nsEventQueueServiceImpl.

Meanwhile, we're trying to run a modal event loop, which needs to process
PLEvents. The timer thread trying to post a timer events, and is calling
nsEventQueueServiceImpl::GetThreadEventQueue, which is waiting to enter that
same monitor. We deadlock, because we never leave the modal event loop that is
causing the mEventQMonitor to be held on the stack of the main thread.

I think this is really a bug for danm. It seems wrong that
PL_ProcessPendingEvents is ever called while the event queue service is the
mEventQMonitor, because you never know whether code called from a plevent is
going to need to do event queue stuff.

Comment 13

[Simon Fraser [no longer active]]

->danm

Comment 14

[Polly]

On eMac with Sys 10.1.5.  Nightly build of Mozilla for Nov 30. 

This bug or something like it came back again in mid-November, 2003.  
Alerts of almost any kind - server-side, or Mozilla password manager, 
Mozilla "do you want to close all tabs" made Mozilla hang. Depending on 
the particulars, I had to quit or force-quit.  Neither "ok" nor "cancel" would 
do anything and in some cases the entire menubar is dimmed while it 
waits. 

Tried it just now and the Mozilla alerts for password choice and tab 
closing worked just fine! However, all is not quite right - a side effect I've 
noticed has occurred - my animated .gif is no longer animated, nor is the 
mozilla icon at the top right  showing any activity when a page is loading.  

The server-side alert hangs Mozilla if I've already lost the animation, but at 
least force-quit wasn't required.  
The code for the alert I used, if this is helpful at all,  is:
function renamepage(){
var pagename = prompt("Please type in a new name for your page?","");
if(pagename){
_cfainvokecontrolhandlerevent('editbar.renamepage',pagename,'editbar');
}

Tell me if I should use another bug to track the bug I've described.  Just 
now I can't make it fail again.   I wanted to spot the moment the animation 
stopped.  Maybe keeping an animated gif in view is a way to prevent the 
bug!  ha ha.

Polly

Comment 15

[Brendan Eich [:brendan]]

Attachment: proposed fix, plus comment cleanup

Not sure why a monitor is needed rather than a lock.  Could use automon/lock
love in 1.7a, too.  But this ought to fix things for 1.6b.

/be

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 136615 [details] [diff] [review]
proposed fix, plus comment cleanup

Thanks to sfraser for the analysis.

/be

Comment 17

[Brendan Eich [:brendan]]

danm: I hope no event handler depends on the thread-id-keyed table entry in the
event queue service still being present; PushThreadEventQueue will cope with the
lack of an entry.

/be

Comment 18

[Darin Fisher]

Comment on attachment 136615 [details] [diff] [review]
proposed fix, plus comment cleanup

looks right to me.  always good to avoid making callbacks while holding a lock!
;)

Comment 19

[Dan M]

Comment on attachment 136615 [details] [diff] [review]
proposed fix, plus comment cleanup

Should be OK. I believe a thread's eldest queue is never disposed of by
popping, so the chewy nugget of this code should never be triggered, anyway. If
it is somehow triggered and we try to push a new eldest queue in some
downstream event handler, well, that'll work as far as we know. And we've got
an assertion there waiting already.

Comment 20

[Brendan Eich [:brendan]]

Fixed.

/be

Comment 21

[benc]

VERIFIED: mozilla 1.6.f, mac os x.