625302 - Add JS API to trigger cycle collection (without garbage collection)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Jesse Ruderman]

The patch in bug 624549 made it possible for the cycle collector to run alone, but there is no way for tests (e.g. fuzzers) to make this happen explicitly.

Please add an API for triggering cycle collection alone. (Preferably on Components, not on nsDOMWindowUtils, so I can trigger cycle collection without having a window reference.)

Comment 1

[Jesse Ruderman]

Is it likely that CC not immediately preceded by GC will cause problems (e.g. crashes) due to violating CC assumptions)? If so, it's important to be able to test.

Comment 2

[Johnny Stenback (:jst)]

Not going to hold the release for this, but I'd approve a patch for this any time.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Patch coming.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Um, actually no.
http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/nsXPConnect.cpp#476 has appeared at some point.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This is anyway the patch, but cann't be used reliably because of that
nsXPConnect thing.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Ben, why exactly is the NS_RUNTIMEABORT needed there?

Comment 7

[Andrew McCreight [:mccr8]]

The CC looks at mark bits that the JS GC produces, so it has to be run after the GC has been run at least once.  That said, the CC checks at the start of its run (before nsXPConnect::BeginCycleCollection) and runs the GC if it hasn't been run yet, so I'm not sure what is happening.

Comment 8

[Andrew McCreight [:mccr8]]

Is nsXPConnect::BeginCycleCollection being called from nsCycleCollector::BeginCollection?  Could the way that the CC invokes the GC not increment the count that nsXPConnect::BeginCycleCollection looks at?

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to comment #7)
> That said, the CC checks at the start of
> its run (before nsXPConnect::BeginCycleCollection) and runs the GC if it
> hasn't been run yet, so I'm not sure what is happening.

AFAIK that's not true, where is that happening?

Comment 10

[Andrew McCreight [:mccr8]]

In nsCycleCollector::BeginCollection, which invokes nsXPConnect::Collect(), which invokes JS_GC.

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp#2565

Comment 11

[Peter Van der Beken [:peterv]]

But that's only for "synchronous" cycle collections, which shouldn't be happening?

Comment 12

[Peter Van der Beken [:peterv]]

(In reply to comment #11)
> But that's only for "synchronous" cycle collections, which shouldn't be
> happening?

The call to nsCycleCollector::BeginCollection should be coming from http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp#3322 which passes false for aForceGC.

Comment 13

[Andrew McCreight [:mccr8]]

Right, but if the GC hasn't been run at all, shouldn't mFirstCollection be true, and thus aForceGC will be set to true?

Comment 14

[Peter Van der Beken [:peterv]]

Ah, yes. Smaug, are you actually hitting the abort?

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Yes, I did hit the abort when I was running the chrome test.

Comment 16

[Andrew McCreight [:mccr8]]

I also hit it when I tried it out.  I haven't quite worked out debugging a Mochitest yet so I'm not sure exactly what is happening. ;)

Comment 17

[Andrew McCreight [:mccr8]]

I did some printf debugging.  BeginCollection behaves the way I expected, and gets into the block that calls Collect().  I'll try looking into whether the collector is actually called or not inside Collect.

Comment 18

[Andrew McCreight [:mccr8]]

The JS GC is not actually triggering.  The cycle collector's invocation of the JS GC gets as far as js_GC in jsgc.cpp, but then it returns from this block of code:

> if (JSGCCallback callback = rt->gcCallback) {
>     if (!callback(cx, JSGC_BEGIN) && gckind != GC_LAST_CONTEXT) {
>         return;
>     }
> }

gckind is a constant GC_NORMAL that is not equal to GC_LAST_CONTEXT, so the callback must be returning false.  Maybe the callback is DOMGCCallback?  I don't know enough about the JS GC to figure out how this works, or why the callback decided not to run the GC.  I looked at it a little bit, but I couldn't really make much sense of it.

Comment 19

[Andrew McCreight [:mccr8]]

My patch in Bug 663532 should fix this, or at least, when applied test_cyclecollector.xul passes.  So when that lands, you should be good to go.

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Testing if this works now...

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Seems to work

Comment 22

[Andrew McCreight [:mccr8]]

As an aside, test_cyclecollector.xul tests the problem in Bug 663532 when run alone, but only if it is run by itself.  If it is in the middle of a bunch of other tests, then the JS GC will probably have run by other means, and the manual JS GC trigger won't be tested.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

review ping.

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/121904a67387

Comment 25

[Mihaela Velimiroviciu (:mihaelav)]

Can anyone please provide some guidelines to verify this fix? Is it necessary to create any specific environment to test it?

Thanks!

Comment 26

[Olli Pettay [:smaug][[e-mail]]]

The patch has a test for this, and the test is being run all the time,
so would say that is enough verify that this is fixed.

Though, one could argue that there should be still some way to test that
GC doesn't run when CC is called.

Comment 27

[Mihaela Velimiroviciu (:mihaelav)]

Considering comment26 setting resolution to VERIFIED-FIXED