157434 - Faulty resize handler triggers infinite recursion in JS engine

Status: VERIFIED DUPLICATE of bug 96128

(Core :: JavaScript Engine, defect)

Description

[Bryan Blackburn]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.1a+) Gecko/20020713
BuildID:    2002071303

Start Mozilla, open
http://www.webengr.com/development/tools/openbsd/tips/upgrading/ then use Cmd-T
to create a new tab.  Mozilla crashes.

Reproducible: Always
Steps to Reproduce:
1. Start Mozilla
2. Open http://www.webengr.com/development/tools/openbsd/tips/upgrading/
3. Cmd-T for new tab
4. Crash

Actual Results:  Browser crashes

Expected Results:  Browser no crash

Comment 1

[Matthias Versen [:Matti]]

Reporter:
We need a stack trace.

Under OS X, you can enable CrashReporter to provide this data. See
[http://developer.apple.com/qa/qa2001/qa1057.html].

Can you please attach the OS X crash log ?

Comment 2

[Bryan Blackburn]

Attachment: Crash log

Comment 3

[Will Jenkins]

Attachment: Mozilla Crash Log

Comment 4

[Will Jenkins]

Mozilla consistently crashes for me, Crash Log attached. Mozilla Build
2002071303 Mac OS X 10.1.5

Comment 5

[Matthias Versen [:Matti]]

this looks like a loop in JS.

Phil: Can you help in this case ?

Comment 6

[Phil Schwartau]

This site relies on the "HierMenus" menu template. Hence we see this
near the top of  the HTML, as with other sites that use this template:

<SCRIPT LANGUAGE="JavaScript1.2" SRC="/heirmenu/HM_Loader.js"></SCRIPT>

Note this script document.writes() two more JS files into the HTML:

                         HM_Arrays.js
                         HM_ScriptDOM.js


The exact files it will load depend on what browser you're using.
(If you're using NN4, it loads HM_ScriptNS4.js, not HM_ScriptDOM.js)

The problem is, this site has made an error and has included the
HM_Loader.js script TWICE! We see it again at the BOTTOM of the page:

<SCRIPT LANGUAGE="JavaScript1.2" SRC="/heirmenu/HM_Loader.js"></SCRIPT>


I experimented by saving the HTML for the site locally, adding
this tag at the top: <base href="http://www.webengr.com">. 

I found that by deleting the second HM_Loader.js <SCRIPT>, the crash
goes away reliably. That doesn't prove anything, but maybe it's a clue.

Reassigning to Embedding:Docshell for a look; cc'ing rginda in case he
knows immediately why loading the same external script at the top and 
bottom of the file might cause this type of crash when the user opens
a new tab.

Comment 7

[Phil Schwartau]

Changing Platform, OS from Mac to All, since I crash
with Mozilla debug build 2002-06-30 on WinNT.

I think my theory is correct. The following reduced testcase
crashes my debug build after I load it and try to open a new tab:


<base href="http://www.webengr.com">
<SCRIPT LANGUAGE="JavaScript1.2" SRC="/heirmenu/HM_Loader.js"></SCRIPT>
<SCRIPT LANGUAGE="JavaScript1.2" SRC="/heirmenu/HM_Loader.js"></SCRIPT>


I will attach this below. Again: if I delete the second, redundant
HM_Loader.js <SCRIPT>, the crash goes away -

Comment 8

[Phil Schwartau]

Attachment: Reduced testcase

Comment 9

[Adam Lock]

Win32 stack trace

<infinite loop & stack overflow>
js_Interpret(JSContext * 0x03adffc0, long * 0x0012e58c) line 2803 + 15 bytes
js_Invoke(JSContext * 0x03adffc0, unsigned int 0x00000000, unsigned int
0x00000000) line 856 + 13 bytes
js_Interpret(JSContext * 0x03adffc0, long * 0x0012f388) line 2803 + 15 bytes
js_Invoke(JSContext * 0x03adffc0, unsigned int 0x00000001, unsigned int
0x00000002) line 856 + 13 bytes
js_InternalInvoke(JSContext * 0x03adffc0, JSObject * 0x03ad1928, long
0x03e653a8, unsigned int 0x00000000, unsigned int 0x00000001, long * 0x0012f5e4,
long * 0x0012f4b4) line 931 + 20 bytes
JS_CallFunctionValue(JSContext * 0x03adffc0, JSObject * 0x03ad1928, long
0x03e653a8, unsigned int 0x00000001, long * 0x0012f5e4, long * 0x0012f4b4) line
3431 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x03adfe28, void * 0x03ad1928,
void * 0x03e653a8, unsigned int 0x00000001, void * 0x0012f5e4, int * 0x0012f5e8,
int 0x00000000) line 1041 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x03de3418, nsIDOMEvent
* 0x03f100d8) line 182 + 77 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x03ddd3f0,
nsIDOMEvent * 0x03f100d8, nsIDOMEventTarget * 0x03adfd20, unsigned int
0x00000002, unsigned int 0x00000007) line 1182 + 20 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x03de23d8,
nsIPresContext * 0x03cf4f58, nsEvent * 0x0012fd10, nsIDOMEvent * * 0x0012fcd0,
nsIDOMEventTarget * 0x03adfd20, unsigned int 0x00000007, nsEventStatus *
0x0012fd38) line 1915 + 36 bytes
GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x03adfd10,
nsIPresContext * 0x03cf4f58, nsEvent * 0x0012fd10, nsIDOMEvent * * 0x0012fcd0,
unsigned int 0x00000001, nsEventStatus * 0x0012fd38) line 766
PresShell::FireResizeEvent() line 3080
PresShell::sResizeEventCallback(nsITimer * 0x03ed8af8, void * 0x03afa988) line 3060
nsTimerImpl::Fire() line 367 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x016d4ca0) line 591
nsAppShell::Run(nsAppShell * const 0x0119a8c0) line 156
nsAppShellService::Run(nsAppShellService * const 0x01197960) line 472
main1(int 0x00000001, char * * 0x00276d00, nsISupports * 0x00000000) line 1508 +
32 bytes
main(int 0x00000001, char * * 0x00276d00) line 1869 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e8d326()

Comment 10

[Adam Lock]

Initial investigation suggests some kind of wacky race condition. Both copies of
the script register themselves with the window.onresize event handler and store
the old resize handler in HM_f_OtherResize.

So when a new tab is opened, a resize event is generated and one of the scripts
handles the resize event in its HM_f_ResizeHandler (in HM_ScriptDOM). It then
calls the old resize handler HM_f_OtherResize. Perhaps somehow these two copies
of the script have managed to store *each other* in their respective
HM_f_OtherResize values causing the resize event to ping pong between the two
until the stack overflows.

Can someone versed with the JS internals let me know if this kind of thing is a
possibility?

Comment 11

[Phil Schwartau]

cc'ing people to consider that question -

Comment 12

[Brendan Eich [:brendan]]

Probably DOM, but what sequence of stack frames recurs in the infinite recursion?

/be

Comment 13

[Adam Lock]

Attachment: Win32 Stack Trace

This is the full Win32 stack trace showing recursion until the crash.

Comment 14

[Adam Lock]

I've step debugged and the recursion begins on line 778 of
http://www.webengr.com/heirmenu/HM_ScriptDOM.js. This is the start of
HM_f_ResizeHandler() and the recursion just goes around and around forever in
this function.

If a script is included twice, does each have it's own scope or do they share
one? Upon loading, each stores the old window.onresize in a global
HM_f_OtherResize and installs its own one. Like this:

  HM_f_OtherResize = (window.onresize) ? window.onresize :  new Function;
  window.onresize = HM_f_ResizeHandler;

If the scopes are shared then on the second invocation HM_f_OtherResize will be
get window.onresize which already points to HM_f_ResizeHandler (since itset the
first time around). Thus when HM_f_ResizeHandler() is finished it calls
HM_f_OtherResize which is also HM_f_ResizeHandler() and spins for ever.

This all depends if the scopes are shared or not. If they are, then what should
be done about it? If not, what else can it be?

Comment 15

[Adam Lock]

Speaking to Rob it appears that the scope is shared so these two copies stomp on
each others globals and cause the infinite loop as described. I'll reassign to
the js engine to determine it isn't catching this before it kills the browser.

Comment 16

[Brendan Eich [:brendan]]

Scripts loaded in a single scope will contend for property names in that scope.
 The testcase isn't ECMA-compliant from what I can tell from adamlock's
analysis.  It might have worked in pre-ECMA versions of JS (Nav2-3, some Nav4
versions).

/be

Comment 17

[Adam Lock]

Shouldn't the JS engine at least catch the recursion even if the page is broken?

Comment 18

[Matthias Versen [:Matti]]

stupid spam comment:
no page should be able to cause a crash in Mozilla.
It's o.k. if a broken page doesn't work but it should no cause a stack overflow.

Comment 19

[Phil Schwartau]

The point Adam and Matti raise is covered by bug 13350,
"DOM needs to police JS infinite loops..."

Reopening this bug to dupe it -

Comment 20

[Phil Schwartau]

Reassigning to DOM Level 0 for parity with bug 13350 -

Comment 21

[Phil Schwartau]

And duping - thanks to all!

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

Comment 22

[Adam Lock]

This isn't just an infinite loop though, it's death by recursion.

Comment 23

[Matthias Versen [:Matti]]

Adam: 
will bug 13350 catch also this problem ? 
We should reopen this bug if not. 
I don't know anything about the code (I'm stupid) but this must be fixed someday.
IMHO resolving as "invalid" is invalid :-)

Comment 24

[Brendan Eich [:brendan]]

If this is an iloop, pschwartau dup'ed it well.  But if this is about a crash
due to runaway recursion, then it seems to me it's a dup of bug 96128.

/be

Comment 25

[Phil Schwartau]

Oops, this is infinite recursion; I'd better dupe it against bug 96128
as Brendan says. Reopening bug in order to do that -

Comment 26

[Phil Schwartau]

Reassigning to JS Engine for parity -

Comment 27

[Phil Schwartau]

Resolving as duplicate -

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

Comment 28

[Phil Schwartau]

Verified Duplicate -