Closed
Bug 709160
Opened 13 years ago
Closed 13 years ago
ObjShrink broke nsXPConnect::Traverse's cycle collector dumping code
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
See for instance this crash report:
https://crash-stats.mozilla.com/report/index/c521651d-6b07-442f-86a3-12ecc2111208
The problematic code is here:
JSFunction* fun = (JSFunction*) xpc_GetJSPrivate(obj);
JSString* str = JS_GetFunctionId(fun);
I could fix that, but I think it is better to just tear out this part of the code and use a JS engine toString function.
Also, I need to add a crash test for cycle collector dumps.
Assignee | ||
Comment 1•13 years ago
|
||
Doing this right will require fixing bug 682353, so I think I'll just do a quick fix to get cycle collector dumps working again, and I'll file a new bug for factoring things out.
Also, looking at MXR this is the only instance in the code where the result of xpc_GetJSPrivate is cast to a JSFunction, so there shouldn't be any other instances of this problem.
Assignee | ||
Comment 2•13 years ago
|
||
Problem 2:
JSObject *global = JS_GetGlobalForObject(NULL, static_cast<JSObject*>(p));
This line seems to crash in what looks like in an assertion due to trying to deref the NULL. When I change NULL to cx, the cycle collector context, then about 1.4mb into the log file it crashes with an assertion failure isGlobal() apparently because the global it finds isn't actually a global.
There's also weird stuff like this in the cycle collector log:
0x11a11dd30 [gc.marked] JS Object (nsXPCComponents_Classes) (global=1185dc420)
> 0x11a11dd00 type_proto
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
> 0x1185dc420 parent
(etc.)
I don't know if that is a problem with JS_TraceChildren or what.
Assignee | ||
Comment 3•13 years ago
|
||
This fixes a few problems that make cycle collector dumping crash, due to fallout from ObjShrink.
1. A JSObject's function should be extracted via JS_GetObjectFunction, as it isn't in the private slot any more.
2. GetGlobalForObject is failing various asserts now, and I haven't been able to debug it, so I've comment out the part that prints the global. That isn't ideal, but it is better than the CC dump crashing.
I'm not really sure what the best way to review this is. The first change is pretty non-controversial, but I'd like to get peterv's (or some XPConnect peer's) super review, if you will, on disabling this part of the CC dumping...
Assignee: nobody → continuation
Attachment #580596 -
Flags: review?(luke)
Attachment #580596 -
Flags: feedback?(peterv)
Assignee | ||
Comment 4•13 years ago
|
||
I checked that cycle collector dumping doesn't crash with this patch applied, at least for some basic browsing.
Updated•13 years ago
|
Attachment #580596 -
Flags: review?(luke) → review+
Assignee | ||
Updated•13 years ago
|
Blocks: ObjectShrink
Assignee | ||
Comment 6•13 years ago
|
||
Target Milestone: --- → mozilla11
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 580596 [details] [diff] [review]
patch around ObjShrink fallout
I landed this to get it in a state where it isn't crashing. I'm not sure how important having the global of each object is. And it isn't clear that it can be safely computed in a post-ObjShrink world...
Attachment #580596 -
Flags: feedback?(peterv)
Assignee | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•