Last Comment Bug 450633 - "script stack space quota is exhausted" exception in JSON.jsm when calling SessionStore API for sessions with a large amount of data
: "script stack space quota is exhausted" exception in JSON.jsm when calling Se...
Status: RESOLVED WONTFIX
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 1.9.0 Branch
: All All
: -- normal (vote)
: mozilla1.9.1b1
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 463399 (view as bug list)
Depends on: 461048 462774
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-14 11:27 PDT by Michael Kraft [:morac]
Modified: 2009-11-19 15:41 PST (History)
16 users (show)
samuel.sidler+old: blocking1.9.0.4-
gavin.sharp: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
split the string in JSON.jsm when it's too large (backed out) (2.07 KB, patch)
2008-08-14 12:19 PDT, Simon Bünzli
sayrer: review+
brendan: superreview+
samuel.sidler+old: approval1.9.0.4-
Details | Diff | Review

Description Michael Kraft [:morac] 2008-08-14 11:27:25 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

If the current session state contains a very large amount of data, then any call to one of the SessionStore API functions that calls the _toJSONString() will result in a "script stack space quota is exhausted" exception being thrown on line 176 in JSON.jsm.

The exception occurs because the javascript "test" function will fail in Firefox 3.0.x when run on strings greater than approximate 1.2 million characters in length.  See bug 420869 for more details about RegExp functions and the stack space quota.

Reproducible: Always

Steps to Reproduce:
The easiest way to generate a large amount of session data is as follows:
1. Set browser.sessionstore.postdata to -1 
2. Upload a 1 MB or greater file using a HTML form (briefcase.yahoo.com works well)
3. In the error console type:  Components.classes["@mozilla.org/browser/sessionstore;1"].getService(Components.interfaces.nsISessionStore).getBrowserState()
Actual Results:  
Error: script stack space quota is exhausted
Source File: file:///C:/Program%20Files/Mozilla%20Firefox/modules/JSON.jsm
Line: 176

Error: uncaught exception: [Exception... "'[JavaScript Error: "script stack space quota is exhausted" {file: "file:///C:/Program%20Files/Mozilla%20Firefox/modules/JSON.jsm" line: 176}]' when calling method: [nsISessionStore::getBrowserState]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: javascript:%20Components.classes["@mozilla.org/browser/sessionstore;1"].getService(Components.interfaces.nsISessionStore).getBrowserState() :: <TOP_LEVEL> :: line 1"  data: yes]

Expected Results:  
No exception should be thrown.

The best method I can think of to fix this would be to break up large strings into multiple smaller ones (less 1,000,000 characters should work), then do the replace and test functions on each of the smaller strings.

The other choice is to just add a try catch around the the test() and replace() function calls in JSON.jsm line 176.  This could result in unsafe JSON strings though.

See also http://forums.mozillazine.org/viewtopic.php?p=4168125#p4168125
Comment 1 Simon Bünzli 2008-08-14 12:19:40 PDT
Created attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

Not sure if this is the right thing to do. I'd like to get this patch on the 1.9.0 branch as well, though, so as not to unnecessarily cripple unaware extensions.
Comment 2 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-08-14 12:20:50 PDT
If the script stack quota is exhausted, Spidermonkey has been _told_ by the application (Gecko here) to not permit more stack allocation.  This isn't a Spidermonkey bug, though it might be either a session-store bug (don't try to process the world at once) or a Gecko bug (allow more stack use, if we can know it's safe to do so).

You seem to agree, since you're not patching the JS engine?
Comment 3 Simon Bünzli 2008-08-14 12:41:00 PDT
Allowing more stack use would IMHO just postpone the issue as there could always be a bigger JSON string to parse (by SessionStore or any other caller) - so patching JSON.jsm looks like the right thing to do.

If this is the wrong component, where should this bug live, then (as JSON.jsm resides under /js)?
Comment 4 Brendan Eich [:brendan] 2008-08-19 09:25:03 PDT
Simon, I'm giving you this bug since you are kindly patching it. It's an xpconnect bug, for better or worse, since xpconnect is (mis-)located under js/src but not part of the core JS engine. Thanks,

/be
Comment 5 Simon Bünzli 2008-08-19 09:39:34 PDT
Comment on attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

Thanks for the info, Brendan. Over to Mike, then.
Comment 6 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-08-19 12:18:10 PDT
Comment on attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

I'm not sure why this is in XPConnect as well, but my usual plan of deflecting to the original author of the code won't work in this case, so I'm going to try our other JSON expert here.

(If it were xpconnect code proper, I'd not have expected sspitzer's review to have carried it into the tree the first time. :) )
Comment 7 Simon Bünzli 2008-08-20 02:16:04 PDT
Thanks for the reviews.
Comment 8 Dão Gottwald [:dao] 2008-09-01 23:37:36 PDT
http://hg.mozilla.org/mozilla-central/rev/38f03dcc5e22
Comment 9 Simon Bünzli 2008-09-02 13:29:37 PDT
Comment on attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

Drivers: This bug breaks session managing extensions when they're most needed: when the session is big and really does need managing.

This patch is of low overall impact, only affecting consumers of JSON.jsm, and thus of low risk. Tests are included and it's been baking on Trunk since 9/02.
Comment 10 Daniel Veditz [:dveditz] 2008-10-01 15:47:21 PDT
Comment on attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

Approved for 1.9.0.4, a=dveditz for release-drivers
Comment 11 Reed Loden [:reed] (use needinfo?) 2008-10-20 19:19:02 PDT
CVS HEAD:

Checking in js/src/xpconnect/loader/JSON.jsm;
/cvsroot/mozilla/js/src/xpconnect/loader/JSON.jsm,v  <--  JSON.jsm
new revision: 1.4; previous revision: 1.3
done
Checking in js/src/xpconnect/tests/unit/test_json.js;
/cvsroot/mozilla/js/src/xpconnect/tests/unit/test_json.js,v  <--  test_json.js
new revision: 1.4; previous revision: 1.3
done
Comment 12 Simon Bünzli 2008-10-25 09:34:28 PDT
To my embarrassment, this patch can lead to a significant performance issue and even broken behavior (as reported in bug 461048) due to the fact that it might split strings in the middle of an escaped character (e.g. the JSON representation of |["\""]| might get split in halves, leading to |["\| and |""]| being checked which will lead to the rejection of a perfectly valid JSON encoded string).

I'd have a patch for fixing this in attachment #344750 [details] [diff] [review], but instead of risking further (even more subtle) breakage, I'd rather back this patch out, leave the bug on the 1.9.0 branch and fix bug 407110 and remove JSON.jsm from Trunk to get rid of this issue.

Extension authors being hit by this bug can always fix the issue for themselves by importing JSON.jsm and then setting JSON.isMostlyHarmless to the function from attachment #344750 [details] [diff] [review] before using the SessionStore API.

Sorry for the hassle!
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-10-25 14:11:07 PDT
https://hg.mozilla.org/mozilla-central/rev/cda625bb27d4

mozilla/js/src/xpconnect/loader/JSON.jsm 	1.5
mozilla/js/src/xpconnect/tests/unit/test_json.js 	1.5
Comment 14 Samuel Sidler (old account; do not CC) 2008-10-25 15:06:46 PDT
Comment on attachment 333803 [details] [diff] [review]
split the string in JSON.jsm when it's too large (backed out)

Minusing for now. We'll want a rolled up patch when this is fixed.
Comment 15 Simon Bünzli 2008-10-27 11:20:21 PDT
(In reply to comment #14)
> Minusing for now. We'll want a rolled up patch when this is fixed.

Do we really want a patch for this at all, considering the stricter branch rules and the fact that a work-around exists?
Comment 16 Samuel Sidler (old account; do not CC) 2008-10-27 11:40:22 PDT
You're right. We don't want this and it doesn't block.
Comment 17 Henrik Skupin (:whimboo) 2008-10-27 14:38:21 PDT
Simon, does it need a dev-doc update to inform extension developers?
Comment 18 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2008-11-06 08:32:47 PST
*** Bug 463399 has been marked as a duplicate of this bug. ***
Comment 19 Henrik Skupin (:whimboo) 2008-12-07 03:47:14 PST
Simon, can you please give an update about the state of this bug?
Comment 20 Simon Bünzli 2008-12-07 04:16:58 PST
This bug is INVALID on Trunk (JSON.jsm was removed in bug 462774) and WONTFIX on the 1.9.0 branch (see comment #16).
Comment 21 Henrik Skupin (:whimboo) 2008-12-07 05:35:42 PST
(In reply to comment #20)
> This bug is INVALID on Trunk (JSON.jsm was removed in bug 462774) and WONTFIX
> on the 1.9.0 branch (see comment #16).

The version field is set to 1.9.0 branch. So lets mark it as WONTFIX.
Comment 22 Eric Shepherd [:sheppy] 2009-11-19 12:44:40 PST
Updated:

https://developer.mozilla.org/En/Using_native_JSON
Comment 23 Dão Gottwald [:dao] 2009-11-19 12:50:05 PST
(In reply to comment #22)
> Updated:
> 
> https://developer.mozilla.org/En/Using_native_JSON

That article doesn't seem to be about the JSON module.
Comment 24 Eric Shepherd [:sheppy] 2009-11-19 12:56:10 PST
OK, we don't have docs for this code module, which has been deprecated anyway. I've yanked the stuff out of that other article and am debating whether or not this justifies actually being written about now.
Comment 25 Dão Gottwald [:dao] 2009-11-19 15:41:55 PST
The code module hasn't only been deprecated but removed. I don't think anything needs to be documented here.

Note You need to log in before you can comment on or make changes to this bug.