Closed
Bug 1214346
Opened 9 years ago
Closed 6 years ago
JSON.stringify should throw a catchable exception in case of OOM
Categories
(Core :: JavaScript: Standard Library, defect)
Core
JavaScript: Standard Library
Tracking
()
RESOLVED
INVALID
People
(Reporter: Yoric, Unassigned)
References
(Blocks 1 open bug)
Details
We probably already have hidden issues (dataloss) with Session Restore due to passing huge objects to JSON.stringify. Making the error catchable will let us at least report it, and behave intelligently.
Reporter | ||
Comment 1•9 years ago
|
||
Reproducible testcase: js -e 'oomAfterAllocations(10); try { JSON.stringify({"a": {"b": {"c": 3}}}) } catch (e) { print("caught") }; print("Yay!")'
Comment 2•7 years ago
|
||
Bump. Jason, is this something your team is willing to pick up, perhaps?
Flags: needinfo?(jorendorff)
Comment 3•6 years ago
|
||
OOM in JSON.stringify() throws the string "out of memory". The test case in comment 1 fails because `oomAfterAllocations` causes all attempts to allocate memory to fail, not just inside JSON.stringify but afterwards as well. So what happens -- we throw "out of memory", try to enter the catch-block, but something in the interpreter has to allocate some memory to run that code (perhaps an environment object? I didn't check), and *that* fails, so instead of running the catch block we throw "out of memory" again. The script terminates with the uncaught exception, and we error out a third time trying to format the exception to print it. That's my guess, at least. Change it to `oomAtAllocation` and it works: oomAtAllocation(10); try { JSON.stringify({"a": {"b": {"c": 3}}}); } catch (e) { print("caught"); } print("Yay!"); Prints: caught Yay! The `oomTest` builtin is basically like a loop around oomAtAllocation(i) or oomAfterAllocation(i): oomTest( () => JSON.stringify({"a": {"b": {"c": 3}}}), { expectExceptionOnFailure: true, keepFailing: true // or false, for oomAtAllocation behavior } ); print("ok"); This also passes.
Flags: needinfo?(jorendorff)
Comment 4•6 years ago
|
||
Sorry, this got put onto an "I should really investigate this" pile and forgotten. I think this is not really a bug and the implications mentioned in comment 0 are not happening; --> RESO INVALID.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
Comment 5•6 years ago
|
||
That's a relief to hear :) Thanks, Jason!
You need to log in
before you can comment on or make changes to this bug.
Description
•