Closed
Bug 800631
Opened 12 years ago
Closed 12 years ago
Remove JSContext arg from JSDebugErrorHook
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
INVALID
People
(Reporter: terrence, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
The problem here is that this is called by js_ReportOutOfMemoryError. If we allow this hook to trigger GC, then effectively cx/rt->malloc can trigger GC. This would be bad, since we currently assume they do not GC.
Currently this hook is unused, so there is no current GC hazard, but we should prevent it while doing so is still easy.
We explicitly forbid GCs during that hook to make it impossible for GCs to happen during malloc. There's a flag called rt->inOOMReport for this purpose.
Reporter | ||
Comment 2•12 years ago
|
||
Looks like we'll be able to remove the hook entirely once JSD goes away. For now, we can guard with our fancy new assertions that this property holds.
Attachment #670936 -
Flags: review?(wmccloskey)
Reporter | ||
Comment 3•12 years ago
|
||
Also meant to say that we can't drop the |cx| because we store the exception info on JSContext and JSD needs to be able to clear it. Ideally we'd factor out the exception info block of the JSContext into its own structure and pass it around as a pointer, but this is just too much work just for JSD's sake.
Comment on attachment 670936 [details] [diff] [review]
v0
I think this will assert. The debug hook is allowed to do things that invoke GC, but the GC will return immediately. However, the assertion will happen before the early return.
If we really want to do this, we would need a way to temporarily cancel the effect of AutoAssertNoGC. Yuck.
Attachment #670936 -
Flags: review?(wmccloskey)
Reporter | ||
Comment 5•12 years ago
|
||
Looks like we just can't do this.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•