Closed Bug 709160 Opened 13 years ago Closed 13 years ago

ObjShrink broke nsXPConnect::Traverse's cycle collector dumping code

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

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.
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.
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.
Attached patch patch around ObjShrink fallout (deleted) — Splinter Review
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)
I checked that cycle collector dumping doesn't crash with this patch applied, at least for some basic browsing.
Attachment #580596 - Flags: review?(luke) → review+
Blocks: ObjectShrink
Blocks: 709162
Target Milestone: --- → mozilla11
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)
Blocks: 710170
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.

Attachment

General

Created:
Updated:
Size: