Last Comment Bug 512624 - TM: Remove JSStackFrame::dormantNext
: TM: Remove JSStackFrame::dormantNext
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-25 21:00 PDT by Andreas Gal :gal
Modified: 2011-06-07 09:04 PDT (History)
6 users (show)
dsicore: blocking1.9.2-
dsicore: wanted1.9.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Andreas Gal :gal 2009-08-25 21:00:39 PDT

    
Comment 1 Andreas Gal :gal 2009-08-25 21:06:18 PDT
The api for dormant frame chains is unfortunately poorly designed w.r.t. error handling. Looks like it has no way to report an oom error in JS_SaveFrameChain.
Comment 2 Brendan Eich [:brendan] 2009-08-25 23:26:46 PDT
What's the problem?

There is no allocation under JS_SaveFrameChain, but if there were, then null return would mean that JS_ReportOutOfMemory(cx) had already been called by the JS_SaveFrameChain implementation.

/be
Comment 3 Andreas Gal :gal 2009-08-25 23:39:48 PDT
I would like to allocate under JS_SaveFrameChain. The API says:

/*
 * Saving and restoring frame chains.
 *
 * These two functions are used to set aside cx's call stack while that stack
 * is inactive. After a call to JS_SaveFrameChain, it looks as if there is no
 * code running on cx. Before calling JS_RestoreFrameChain, cx's call stack
 * must be balanced and all nested calls to JS_SaveFrameChain must have had
 * matching JS_RestoreFrameChain calls.
 *
 * JS_SaveFrameChain deals with cx not having any code running on it. A null
 * return does not signify an error, and JS_RestoreFrameChain handles a null
 * frame pointer argument safely.
 */

So I don't have a way to signal OOM if we were to implement this with a vector in cx.
Comment 4 Brendan Eich [:brendan] 2009-08-25 23:51:34 PDT
Duh, I see! We should change the API, it's new enough we can get away with it (did it even release in an extant spidermonkey tarball?).

Two options:

1. Change the Save API to return JSBool with a maybe-null JSStackFrame pointer in an out param.

2. (Ugly): change the API to return a non-null invalid value ((JSStackFrame *)NULL + 1) to mean error.

/be
Comment 5 Andreas Gal :gal 2009-08-25 23:55:25 PDT
Why do we return the JSStackFrame pointer at all? Can we just make it bool?
Comment 8 Brendan Eich [:brendan] 2009-08-26 00:03:04 PDT
The return value is for sanity checking -- we want caller to match internal set-aside, to catch unbalanced/mismatched save/restore control flow, etc. This is intentional but perhaps it's not worth the hassle.

/be
Comment 9 Damon Sicore (:damons) 2009-09-17 12:10:33 PDT
Not a blocker.  Enhancement.  Makes recursion faster.  Still should do.
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2011-06-07 09:04:39 PDT
Somebody fixed this sometime.

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