TM: Remove JSStackFrame::dormantNext

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: gal, Unassigned)

Tracking

Trunk
x86
Mac OS X
Points:
---
Bug Flags:
blocking1.9.2 -
wanted1.9.2 +

Firefox Tracking Flags

(Not tracked)

Details

Comment hidden (empty)
(Reporter)

Comment 1

8 years ago
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.
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
(Reporter)

Comment 3

8 years ago
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.
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
(Reporter)

Comment 5

8 years ago
Why do we return the JSStackFrame pointer at all? Can we just make it bool?
(Reporter)

Updated

8 years ago
Flags: blocking1.9.2?
Whiteboard: fixed-in-tracemonkey
(Reporter)

Updated

8 years ago
Whiteboard: fixed-in-tracemonkey
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
Not a blocker.  Enhancement.  Makes recursion faster.  Still should do.
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Somebody fixed this sometime.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.