Components.stack is providing wrong stack trace

RESOLVED WORKSFORME

Status

()

Core
XPConnect
RESOLVED WORKSFORME
6 years ago
2 years ago

People

(Reporter: Honza, Unassigned)

Tracking

Trunk
x86
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [firebug-p1])

(Reporter)

Description

6 years ago
Firebug Console panel is using JSD to get proper stack frames, which is in some cases more precise than just climbing Components.stacks.

Follow this test case:
http://getfirebug.com/tests/content/branches/1.9/console/4658/issue4658.html

... and compare Firebug Console panel result with Web Console (built in) result.

- Firebug is correctly showing: FirebugBug.js (line 6) - based on JSD 
- Web Console is showing: dojo.js.uncompressed.js:510 - based on Components.stack

Could we fix Components.stack so, Web Console works and Firebug Console panel doesn't need to activate JSD (performance penalty) to get correct stack frames?

Honza

Comment 1

6 years ago
Out of curiosity, is the code in question consuming all the stack frames or just the top frame?  I ask because we recently started capping the max depth of the stack traversed by Console.stack (since it was hanging browsers with deep stacks).  In general, Console.stack is really expensive, especially considering that most sites just want Console.stack[0] or [1].
(Reporter)

Comment 2

6 years ago
(In reply to Luke Wagner [:luke] from comment #1)
> Out of curiosity, is the code in question consuming all the stack frames or
> just the top frame?
The top frame is only important in this particular case (console.log),
but in case of e.g. errors (console.error) - Firebug is displaying full stack trace.

> I ask because we recently started capping the max depth
> of the stack traversed by Console.stack (since it was hanging browsers with
> deep stacks).  In general, Console.stack is really expensive, especially
> considering that most sites just want Console.stack[0] or [1].
Console.stack? Do you mean Components.stack?

Honza

Comment 3

6 years ago
(In reply to Jan Honza Odvarko from comment #2)
> Console.stack? Do you mean Components.stack?

Oops, yes.
(Reporter)

Comment 4

6 years ago
Any change we could get this one fixed?
Honza

Updated

6 years ago
Assignee: general → nobody
Component: JavaScript Engine → XPConnect
QA Contact: general → xpconnect
(Reporter)

Comment 5

6 years ago
Could we set target milestone for this problem? Fx 15:
Honza
(Reporter)

Updated

5 years ago
Whiteboard: [firebug-p1]
(Reporter)

Comment 6

5 years ago
Any chance we could get this bug higher priority?
Could we fix it as part of the "remove JSD1" effort? 

Honza
(Reporter)

Updated

5 years ago
Blocks: 800200

Comment 7

5 years ago
Based on a quick look, Components.stack calls nsXPCComponents::GetStack which uses JS::DescribeStack which does proper stack iteration.  I didn't step through the STR, but based on the results in comment 0, I think we are just iterating the stack of the wrong JSContext.  The JSContext is extracted by GetCurrentJSContext() whereas, iirc, jsd is a lot more explicit about JSContext (which I never actually understood).

Perhaps jimb or bholley could look at this?
Yeah, what luke says sounds right.

Honza, can you clarify what you're trying to do here and why this is a blocker for removing JSD1? JSD1 does all kinds of crazy stuff with the JSContext (see bug 871306 for example), but I was hoping we could just let it die rather than messing with it too much. ;-)

If we wanted to fix this, we probably need to push a cx somewhere in JSD. I don't know the code well enough to have much of an idea of where to do that, though.

Does the same problem happen with JSD2?

Comment 9

5 years ago
Bobby: we have the ability to enumerate *everything* live on the stack (regardless of JSContext), which we could provide (perhaps by a bool allTheThings param to JS::DescribeStack).  Do you think that would fix the problem and be generally suitable for Components.stack?
(In reply to Luke Wagner [:luke] from comment #9)
> Bobby: we have the ability to enumerate *everything* live on the stack
> (regardless of JSContext), which we could provide (perhaps by a bool
> allTheThings param to JS::DescribeStack).

(Note that AllFramesIter currently does not support inlined (Ion-)frames. However, as part of the stack overhaul my plan is to merge it with ScriptFrameIter since the difference between the two will be very small: skip activations from other contexts or don't.)
(In reply to Luke Wagner [:luke] from comment #9)
> Bobby: we have the ability to enumerate *everything* live on the stack
> (regardless of JSContext), which we could provide (perhaps by a bool
> allTheThings param to JS::DescribeStack).  Do you think that would fix the
> problem and be generally suitable for Components.stack?

Whoa, what? How do we do that? I think stacks were per-JSContext.

That sounds like it maps reasonably well to the cx-less world, so if it deals with interleaving and all that, maybe we should give that a try.

Comment 12

5 years ago
There is only one actual stack per JSRuntime (rt->stackSpace); all JSContext execution interleaves in a LIFO manner and threads through the same rt->stackSpace.  The normal ScriptFrameIter iterates within a context, but the AllFramesIter walks over the raw rt->stackSpace.  Jan was pointing out some of the current limitations of AllFramesIter, though.

> That sounds like it maps reasonably well to the cx-less world

Definitely; the multi-cx world is just a complication layered on top.
(Reporter)

Comment 13

5 years ago
(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #8)
> Honza, can you clarify what you're trying to do here and why this is a
> blocker for removing JSD1? JSD1 does all kinds of crazy stuff with the
> JSContext (see bug 871306 for example), but I was hoping we could just let
> it die rather than messing with it too much. ;-)

I have been looking at the provided test case again (comment #0) and noticed that it involves @sourceURL directive, which is additionally processed by Firebug. It explains the difference between Firebug Console panel and Web Console.

I have been looking for a better test case and haven't found one so far. Ao, I agree that it shouldn't be a blocker for JSD1 removal (at least till there is a proper test case).

Honza
No longer blocks: 800200
This looks fixed to me in Firefox 41.0. The console displays 'FirebugBug.js:6:9'.

According to mozregression it was fixed somewhere in this push log (couldn't find the related bug, though):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a280a03c9f3c&tochange=ae1dfa192faf

Sebastian
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.