Closed Bug 157434 Opened 18 years ago Closed 18 years ago

Faulty resize handler triggers infinite recursion in JS engine

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED DUPLICATE of bug 96128

People

(Reporter: withay, Assigned: rogerl)

References

()

Details

(Keywords: crash, testcase)

Attachments

(4 files)

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
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 ?
Attached file Crash log
Attached file Mozilla Crash Log
Mozilla consistently crashes for me, Crash Log attached. Mozilla Build
2002071303 Mac OS X 10.1.5
Status: UNCONFIRMED → NEW
Ever confirmed: true
this looks like a loop in JS.

Phil: Can you help in this case ?
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.
Assignee: Matti → adamlock
Component: Browser-General → Embedding: Docshell
QA Contact: asa → adamlock
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 -
OS: MacOS X → All
Hardware: Macintosh → All
Attached file Reduced testcase
Keywords: crash, testcase
Summary: Opening some URLs then creating a new tab causes crash → Opening some URLs then creating a new tab causes crash [@ nsString::SetLength]
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()
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?
cc'ing people to consider that question -
Probably DOM, but what sequence of stack frames recurs in the infinite recursion?

/be
Attached file Win32 Stack Trace
This is the full Win32 stack trace showing recursion until the crash.
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?
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.
Assignee: adamlock → rogerl
Component: Embedding: Docshell → JavaScript Engine
QA Contact: adamlock → pschwartau
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
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → INVALID
Shouldn't the JS engine at least catch the recursion even if the page is broken?
Component: JavaScript Engine → Embedding: Docshell
Summary: Opening some URLs then creating a new tab causes crash [@ nsString::SetLength] → Faulty resize handler triggers infinite recursion in JS engine
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.
The point Adam and Matti raise is covered by bug 13350,
"DOM needs to police JS infinite loops..."

Reopening this bug to dupe it -
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Reassigning to DOM Level 0 for parity with bug 13350 -
Assignee: rogerl → jst
Status: REOPENED → NEW
Component: Embedding: Docshell → DOM Level 0
QA Contact: pschwartau → desale
And duping - thanks to all!

*** This bug has been marked as a duplicate of 13350 ***
Status: NEW → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → DUPLICATE
This isn't just an infinite loop though, it's death by recursion.
Component: DOM Level 0 → Embedding: Docshell
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 :-)
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
Oops, this is infinite recursion; I'd better dupe it against bug 96128
as Brendan says. Reopening bug in order to do that -
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reassigning to JS Engine for parity -
Assignee: jst → rogerl
Status: REOPENED → NEW
Component: Embedding: Docshell → JavaScript Engine
QA Contact: desale → pschwartau
Resolving as duplicate -

*** This bug has been marked as a duplicate of 96128 ***
Status: NEW → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → DUPLICATE
Verified Duplicate -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.