502836 - "script stack space quota is exhausted" error on Large GWT App (5,632KB)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jaco Greeff]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

We are close to launching a new medium-sized application using GWT 1.5.3 + GXT 1.2.4. At this point the application is around 81,000 lines (61K of this is the actual web client, the remainder is the server-side), along with another 4,300 DTO objects used in the RPC (don't ask), translated into 3 languages (600+ strings per language) and weighs in at a (not-too small) 5,632KB compiled size.

Up to a week ago, everything was fine and then FireFox 3.5 happened. (Our main target for our enterprise app is IE and FF, as an ISV that seems to be where the action is.)

At the end of last week, straight after the release of FF 3.5, the application started reporting the following errors when executing: (All other browsers are fine, including FF 3.x, IE 6-8, Safari, Chrome & Opera)

"script too large"

This week (with some code added in the meantime) the original error was replaced by a

"script stack space quota is exhausted"

Judging by these and the work that happened in the JS engine in the FF 3.5 release, I'm starting to assume that this browser release is not too friendly to our sized compiled GWT applications. I'm not too excited by the fact that we will need to ship without FF 3.5 support, but that seems to be the current scenario. 

The application is internally developed, for corporate clients - no URL available for outside access, although code install can be provided for testing.

Reproducible: Always

Steps to Reproduce:
1. Launch application
2. Error appears
3. Application doesn't execute


Expected Results:  
Application worked perfectly before FF 3.5 release, i.e. it is fine in other 3.x releases. This falls into the "regression" category with regards to the new JS engine.

The application is internally developed, for corporate clients - no URL available for outside access, although code install can be provided for testing.

Comment 1

[Tyler Downer [He/Him]]

Well, seems to be more of a core thing, not sure where

Comment 2

[Sylvain Pasche]

-> JS engine

Comment 3

[Samuel Sidler (old account; do not CC)]

sayrer: You nominated this for blocking, but it needs an owner before we'd consider it blocking. I'll mark it wanted for now...

Comment 4

[Jaco Greeff]

Attachment: FF-only testcase

I'm attaching a zip file containing a stripped down version of the actual application in question. This is useful as a test-case. It is not pretty, all images and css is stripped out to save space, but it will show that it is working under older FF 3.0.x releases and not in 3.5.

Execution/testing/vailidation can be done extracting the zip file and launching index.html (If it works, the loading will disappear and "cannot get a list of available databases" message will be displayed.)

Comment 5

[Paul Nickerson]

This same error is inhibiting some of my fuzzer testcase reproduction.

Comment 6

[Paul Nickerson]

I can provide one of the testcases that throws the same error if needed.

Comment 7

[Jaco Greeff]

Paul, I don't think an additional test-case would hurt. There are always some corners cases which might not manifest itself with a single case.

Comment 8

[Paul Nickerson]

Created secure bug 513399 and attached the testcase because it includes protected content

Comment 11

[Jo Hermans]

note that the script stack quota is currently hardcoded to 100 MB (see bug 420869)

Comment 12

[Paul Nickerson]

Why would we hardcode it? Wouldn't it be better to dynamically limit it to the available memory?

Comment 13

[Jo Hermans]

see bug 420869 comment 23 and 28

Comment 14

[Brendan Eich [:brendan]]

So this can be that "other bug" Igor mentioned. It seems worth trying to fix if it is breaking real web apps.

/be

Comment 15

[Brian Amundson]

Our application's size has just reached this threshold as well, and now we are seeing the same errors in the browser. If we run our GWT compile and set the output as OBF we see this issue. If we change the GWT compiler style to PRETTY, we do not see this issue. This is not desirable as it makes it easier to reverse-engineer our application, and compile time skyrockets.

Comment 16

[Danny Moules]

Could this not be a user option?

ie. App requests privilege to ignore the limit upon execution -> User consents in the knowledge it may slow the browser down or rejects the request.

We're having to supply our own build to clients to bypass this issue.

Comment 17

[Alon Zakai (:azakai)]

Attachment: another test case (bullet physics engine)

Attached is a (compressed) 7MB JavaScript file that also runs into this error (unrelated to the GWT application mentioned in this bug, but apparently also hitting the same problem).

The file is the Bullet physics engine (zlib), compiled from C++ into JavaScript. Works on V8, gets

  script stack space quota is exhausted

on latest tracemonkey.

Comment 18

[Alon Zakai (:azakai)]

Attachment: patch

Patch does two things:

1. Bump JS_DEFAULT_SCRIPT_STACK_QUOTA from 32MB to 128MB.
2. Use at least PR_GetPhysicalMemorySize()/8 for JS_SetScriptStackQuota.

With this patch, the testcases in this bug work properly, both in the js shell, and in the browser.

Who can I ask to review this?

Comment 19

[Alon Zakai (:azakai)]

Turns out there is a problem here, a jsreftest fails, regress-336409-2.js.

The issue is that, before this patch, QuoteString will call SprintEnsureBuffer, which will find that there is no room to allocate, and call js_ReportOutOfScriptQuota. This reports an error, an exception is thrown, and the jsreftest is happy.

But with this patch, SprintEnsureBuffer does *not* run into a problem. However there are two potential problems later on,

           if (curlen > size_t(-1) / sizeof(jschar))
                goto overflow;
 
           /* Allocate 1 + 1 at end for closing brace and terminating 0. */
           chars = (jschar *) js_realloc((ochars = chars), curlen * sizeof(jschar));
           if (!chars) {

  http://mxr.mozilla.org/mozilla-central/source/js/src/jsobj.cpp#723

If either of these checks fail (the latter actually does on tryserver), we call js_ReportOutOfMemory - which does *not* throw an exception. Instead, we get 'an error, not a catchable exception',

  http://mxr.mozilla.org/mozilla-central/source/js/src/jsinterp.cpp#6752

The script is halted immediately, silently, without any indication that anything is wrong. In this case the jsreftest is sad.

Can we throw an exception, so we get the same result as before? We are in an out-of-memory situation here, so that is trickier I guess, but what is the downside of trying to report an exception?

Comment 20

[Igor Bukanov]

(In reply to comment #19)
> The script is halted immediately, silently, without any indication that
> anything is wrong. In this case the jsreftest is sad.

Add to the test case expectExitCode(5). This tells to the test suite that the shell process will terminate due to out-of-memory condition with the exit code 5.

Comment 21

[Alon Zakai (:azakai)]

(In reply to comment #20)
> (In reply to comment #19)
> > The script is halted immediately, silently, without any indication that
> > anything is wrong. In this case the jsreftest is sad.
> 
> Add to the test case expectExitCode(5). This tells to the test suite that the
> shell process will terminate due to out-of-memory condition with the exit code
> 5.

Hmm, the jsreftest already had that line - doesn't help.

  http://mxr.mozilla.org/mozilla-central/source/js/src/tests/js1_5/extensions/regress-336409-2.js#48

When running the test in a browser, it simply halts that script, but continues normally otherwise - there is no indication anything wrong happened (except further lines in that particular script are simply not executed). I had to use the debugger to figure out what was actually going on (the 'error but not exception' situation mentioned above). I guess this is offtopic for this bug, but shouldn't an indication be given to the user - like throwing an exception?

Comment 22

[Alon Zakai (:azakai)]

Attachment: patch v2

Changed the memory used to physical memory/4 instead of /8 - any objection?

The reason is I have another testcase, 30MB of JS (the sauerbraten game engine), which requires yet more memory. (It's too big even when bzip2'd to be attached here on bugzilla.)

Comment 23

[Alon Zakai (:azakai)]

http://hg.mozilla.org/tracemonkey/rev/25ba554a24ac

Comment 24

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/25ba554a24ac