90143 - browser crashes after using this menu application

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[dries samyn]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.2) Gecko/20010628
BuildID:    2001062815

In http://www.dries.net/bugMoz.html:

If you hover over the menus, a description should appear. This happens okay, but
mozilla browsers (NS6.01 does is as well) crash after a few times. Try and see ...

Reproducible: Always
Steps to Reproduce:
1.roll over quick
2.roll over quick, repeatidaly
3. ...

Actual Results:  alert sais:
"mozilla.exe has generated errors and will be closed by Windows. You will need
to restart the program.
An error log is being created."

I pasted the drWatson log in the Additional information box

Expected Results:  I would expect that a bit of JavaScript wouldn't be able to
crash a whole browser. Think about the potential ...

Application exception occurred:
        App: mozilla.exe (pid=1252)
        When: 7/10/2001 @ 16:16:24.662
        Exception number: c0000005 (access violation)

*----> System Information <----*
        Computer Name: LONWS0005
        User Name: dsamyn
        Number of Processors: 1
        Processor Type: x86 Family 6 Model 8 Stepping 3
        Windows 2000 Version: 5.0
        Current Build: 2195
        Service Pack: 1
        Current Type: Uniprocessor Free
        Registered Organization: Razorfish Inc.
        Registered Owner: Razorfish UK

*----> Task List <----*
   0 Idle.exe
   8 System.exe
 140 SMSS.exe
 164 csrss.exe
 184 WINLOGON.exe
 212 services.exe
 224 LSASS.exe
 392 svchost.exe
 420 SPOOLSV.exe
 496 DKService.exe
 512 svchost.exe
 556 regsvc.exe
 572 mstask.exe
 596 tcpsvcs.exe
 636 snmp.exe
 328 SWNETSUP.exe
 808 SWEEPSRV.SYS.exe
 820 SWUPDATE.exe
 264 explorer.exe
1056 promon.exe
1096 internat.exe
1028 ICMON.exe
 448 OUTLOOK.exe
1128 MAPISP32.exe
1248 whAgent.exe
1208 AGSatellite.exe
1404 msmsgs.exe
 432 IEXPLORE.exe
1216 homesite45.exe
1252 mozilla.exe
 548 DRWTSN32.exe
   0 _Total.exe

(00400000 - 00456000) 
(77F80000 - 77FFA000) 
(60EC0000 - 60F26000) 
(60D90000 - 60D99000) 
(60E00000 - 60E24000) 
(77DB0000 - 77E0A000) 
(77E80000 - 77F35000) 
(77D40000 - 77DB0000) 
(75050000 - 75058000) 
(75030000 - 75044000) 
(78000000 - 78046000) 
(75020000 - 75028000) 
(60E90000 - 60E97000) 
(60EA0000 - 60EA6000) 
(69800000 - 69A42000) 
(77F40000 - 77F7C000) 
(77E10000 - 77E74000) 
(77C70000 - 77CBA000) 
(77B50000 - 77BD9000) 
(77A50000 - 77B45000) 
(6E420000 - 6E426000) 
(75E60000 - 75E7A000) 
(60020000 - 60025000) 
(60060000 - 60095000) 
(60CC0000 - 60D0C000) 
(600A0000 - 600B0000) 
(600B0000 - 600BC000) 
(600C0000 - 600CD000) 
(600D0000 - 600D5000) 
(600F0000 - 600FF000) 
(60100000 - 6010B000) 
(60110000 - 60125000) 
(60130000 - 60190000) 
(60190000 - 6019B000) 
(601B0000 - 601B6000) 
(601C0000 - 601C5000) 
(601D0000 - 602FC000) 
(60C70000 - 60C84000) 
(60C90000 - 60C9C000) 
(60300000 - 6031C000) 
(76B30000 - 76B6E000) 
(60320000 - 60402000) 
(60410000 - 6044A000) 
(60450000 - 60468000) 
(77820000 - 77827000) 
(759B0000 - 759B6000) 
(60470000 - 60480000) 
(60480000 - 604A0000) 
(779B0000 - 77A45000) 
(77570000 - 775A0000) 
(604A0000 - 604A6000) 
(604B0000 - 604B6000) 
(604C0000 - 604C5000) 
(60CA0000 - 60CB5000) 
(604D0000 - 604D9000) 
(604E0000 - 604EF000) 
(60F40000 - 60F4C000) 
(604F0000 - 604F5000) 
(60570000 - 60579000) 
(60580000 - 605A1000) 
(605B0000 - 605B8000) 
(605C0000 - 605C6000) 
(605D0000 - 605D6000) 
(60600000 - 6061A000) 
(60620000 - 60626000) 
(60630000 - 60636000) 
(60650000 - 60656000) 
(607B0000 - 607B5000) 
(607C0000 - 60802000) 
(60810000 - 6081D000) 
(60820000 - 6082D000) 
(60830000 - 60836000) 
(60840000 - 60845000) 
(60850000 - 6085C000) 
(60860000 - 6087F000) 
(60880000 - 6088F000) 
(60890000 - 60899000) 
(608A0000 - 608AA000) 
(60D10000 - 60D21000) 
(60970000 - 6097C000) 
(60980000 - 60997000) 
(609A0000 - 609A6000) 
(609C0000 - 609C6000) 
(609E0000 - 609E8000) 
(60A00000 - 60A1F000) 
(60A20000 - 60A27000) 
(60A30000 - 60A39000) 
(60A40000 - 60A49000) 
(60A50000 - 60A5E000) 
(60A60000 - 60A7E000) 
(60A80000 - 60A87000) 
(60A90000 - 60ABC000) 
(60AC0000 - 60AD3000) 
(60AE0000 - 60AFC000) 
(60B00000 - 60B1F000) 
(60B20000 - 60B4D000) 
(60B50000 - 60B5B000) 
(60B70000 - 60B81000) 
(60B90000 - 60B9D000) 
(60BA0000 - 60BA5000) 
(60BB0000 - 60BB9000) 
(60BE0000 - 60C02000) 
(60C20000 - 60C41000) 
(60C50000 - 60C5C000) 
(10000000 - 1000A000) 
(74FD0000 - 74FED000) 
(77340000 - 77353000) 
(77520000 - 77525000) 
(77320000 - 77337000) 
(75150000 - 7515F000) 
(75170000 - 751BF000) 
(77BE0000 - 77BEF000) 
(751C0000 - 751C6000) 
(77950000 - 77979000) 
(77980000 - 779A4000) 
(773B0000 - 773DE000) 
(77380000 - 773A2000) 
(77830000 - 7783E000) 
(77880000 - 7790D000) 
(77C10000 - 77C6D000) 
(774E0000 - 77512000) 
(774C0000 - 774D1000) 
(77530000 - 77552000) 
(77360000 - 77379000) 
(691D0000 - 69255000) 
(75010000 - 75017000) 
(77840000 - 7787C000) 
(770C0000 - 770E3000) 
(78280000 - 7828C000) 
(777E0000 - 777E8000) 
(777F0000 - 777F5000) 
...

(It doesn't allow me to paste all of it, let me know if you need all)

Comment 1

[Boris Zbarsky [:bzbarsky]]

I can't reproduce this with a linux build from 2001-07-09

Comment 2

[Matthias Versen [:Matti]]

confirming with win2k build 20010710..
Reporter: Please add the talkback info not direct in the bug, attach it.
(You can attach also a testcase,use the "Create new attachment" link in the bug)
And that is not what we need (if you open a new bug..), we need the Talkback ID# 
that you can get if you run "Install-Dir\components\talkback.exe".

A part of the stack :

GlobalWindowImpl::ClearTimeout(GlobalWindowImpl * const 0x0443dfc4, int 744) 
line 2070
XPTC_InvokeByIndex(nsISupports * 0x0443dfc4, unsigned int 76, unsigned int 1, 
nsXPTCVariant * 0x0012c2a8) line 139
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 1881 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x0447e8c0, JSObject * 0x043c8a00, unsigned int 1, 
long * 0x0530097c, long * 0x0012c4dc) line 1252 + 11 bytes
js_Invoke(JSContext * 0x0447e8c0, unsigned int 1, unsigned int 0) line 807 + 23 
bytes
js_Interpret(JSContext * 0x0447e8c0, long * 0x0012d27c) line 2701 + 15 bytes
js_Invoke(JSContext * 0x0447e8c0, unsigned int 2, unsigned int 0) line 824 + 13 
bytes
js_Interpret(JSContext * 0x0447e8c0, long * 0x0012dfd4) line 2701 + 15 bytes
js_Invoke(JSContext * 0x0447e8c0, unsigned int 1, unsigned int 2) line 824 + 13 
bytes
js_InternalInvoke(JSContext * 0x0447e8c0, JSObject * 0x0476bbe8, long 74890224, 
unsigned int 0, unsigned int 1, long * 0x0012e1ac, long * 0x0012e0fc) line 896 + 
20 bytes
JS_CallFunctionValue(JSContext * 0x0447e8c0, JSObject * 0x0476bbe8, long 
74890224, unsigned int 1, long * 0x0012e1ac, long * 0x0012e0fc) line 3320 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x043dd000, void * 0x0476bbe8, 
void * 0x0476bbf0, unsigned int 1, void * 0x0012e1ac, int * 0x0012e1a8, int 1) 
line 941 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x048ae298, nsIDOMEvent 
* 0x05206cbc) line 139 + 57 bytes
........

Comment 3

[dries samyn]

I expect it has something to do with the timer. I create a timer object in 
order to 'clear' it. This reacts different in other browsers. In the Mozilla 
browsers for example, the application doesn't work if I don't give the timer 
object an initial value.

Comment 4

[Pete Collins (MDG)]

I can't reproduce this on unix (FreeBSD) running a builkd off the tip.
Looks like it may be windows specific.
--pete

Comment 5

[Jerome Lacoste]

I've reproduced the bug with latest mozilla 0.9.4.
Tallback ID: TB35430314G

PS: it should be possible to copy paste the Incident ID # .... from the tallback
client.

Comment 6

[Fabian Guisset]

Crashed on 0.9.5 win2k, talkback ID TB36921398Y

Comment 7

[Fabian Guisset]

I don't crash on linux, neither on 0.9.4 nor on a cvs debug build, thus cannot
get a stack trace :-(

Comment 8

[Johnny Stenback (:jst)]

Attachment: Proposed fix

Comment 9

[Johnny Stenback (:jst)]

Ok, I found one (the?) problem in the timeout code.

While we're executing timeouts in the DOM code we walk the list of timeouts and
execute the ones that are supposed to be executed. As we walk the list of
timeouts we keep a local pointer |prev| to the previous timeout (that wasn't
executed) and we use this local pointer when removing timeouts out of our
one-way linked list of timeouts. The problem is that as a timeout is executed it
can, and does in this case, remove other timeouts from the linked list, and in
this testcase after wiggling the mouse over the menu long enough, we end up in a
situation where the timeout that our local |prev| pointer points to is removed
from the linked list. As we go ahead and remove the current timeout from the
linked list we actually remove the current timeout from the wrong linked list
(the one that's hanging off off the deleted |prev| pointer and not from mTimeouts).

Comment 10

[Johnny Stenback (:jst)]

Well, after some more thinking about this it turns out that what I found was not
the problem, but a result of the problem.

I now found the real reason (I hope) for why |prev| is deleted while we're
executing timeouts (i.e. this should not happen). The reason is that the OS
timers on Win32 (and who knows what happens on other platforms) fire early most
of the time, and some times even out of order, this is the killer. If all the
folowing condition are true we're in trouble:

- a timeout's timer fires too early
- the timer fires before the timer in another timeout that should've fired before
  the other timeout's timer
- the amount of time between the two timeout's is smaller than the amount of time
  that the OS timer fired early

In this case, we'll end up executing timeouts out of order, we'll fire a timeout
in the middle of the list w/o firing the timeouts before that timeout in the
list, and this we can't deal with.

New patch coming up.

Comment 11

[Johnny Stenback (:jst)]

Attachment: Proposed fix

Comment 12

[Peter Van der Beken [:peterv]]

Comment on attachment 54649 [details] [diff] [review]
Proposed fix

r=peterv after a long explanation by jst.

Comment 13

[vidur (gone)]

Comment on attachment 54649 [details] [diff] [review]
Proposed fix

Rather than re-query PR_IntervalNow() in the early case, can we not just hold onto the old value? 

Other than that, sr=vidur.

Comment 14

[Johnny Stenback (:jst)]

Attachment: Vidur's comments addressed...

Comment 15

[Johnny Stenback (:jst)]

Fix checked in on the trunk, this one should go on the branch as well.

Comment 16

[Johnny Stenback (:jst)]

Won't make it onto the branch, marking FIXED.