Closed Bug 614578 Opened 9 years ago Closed 9 years ago
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.
> 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?
(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.
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)
We don't have to block on this, but we should definitely take this if its done in time.
Attachment #493439 - Flags: review?(mrbkap) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.