Closed
Bug 614578
Opened 14 years ago
Closed 14 years ago
Remove nsAutoGCRoot
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status2.0 | --- | wanted |
People
(Reporter: igor, Assigned: igor)
References
(Blocks 1 open bug)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
The conservative GC should take about all users of nsAutoGCRoot. But we should not just remove the class. Rather its usage should be replaced by an auto class that would assert in the constructor and destructor that code executes under the JS request.
Comment 1•14 years ago
|
||
> The conservative GC should take about all users of nsAutoGCRoot.
That's not necessarily true... Consider this code pattern:
JSObject *obj = getMyObject()
MyClass* priv = obj->getPrivate();
// work with priv
Since |obj| is unused after the getPrivate() call, the compiler can reuse its stack slot... but it might be the only thing keeping |priv| alive, no?
Assignee | ||
Comment 2•14 years ago
|
||
(In reply to comment #1)
> Since |obj| is unused after the getPrivate() call, the compiler can reuse its
> stack slot... but it might be the only thing keeping |priv| alive, no?
Very good point. Indeed this is similar to the string/chars issue. I guess we need some form of an autoroot class.
Assignee | ||
Comment 3•14 years ago
|
||
I monitored the code and it turned out that that the pattern from the comment 1 do not exits among nsAutoRoot users. So this patch is based on Gal's patch from the bug comment 5 613319 with extra changes to ensure that the code always runs a JS request when there is a root on the stack.
With the patch the only users of AddRoot functionality is JS_GetPropertyDescArray and extensions/jssh/nsJSSh.cpp where the of AddRoot in the latter can lead to uncollectable loop. But the case of JS_GetPropertyDescArray is a hard one and clearly shows that some form of AddRoot is necessary if we want to avoid the burden of rooting with JSPropertyDescArray callers. But these worries are for the bug 613319.
Assignee: general → igor
Attachment #493439 -
Flags: review?(mrbkap)
Comment 4•14 years ago
|
||
We don't have to block on this, but we should definitely take this if its done in time.
Updated•14 years ago
|
No longer blocks: compartmentGC
Updated•14 years ago
|
Attachment #493439 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 5•14 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Comment 6•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•