614578 - Remove nsAutoGCRoot

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 2

[Igor Bukanov]

(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.

Comment 3

[Igor Bukanov]

Attachment: v1

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.

Comment 4

[Andreas Gal :gal]

We don't have to block on this, but we should definitely take this if its done in time.

Comment 5

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/fc0384edf67b

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/fc0384edf67b