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

VERIFIED FIXED

Status

()

--
enhancement
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: jruderman, Assigned: smaug)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

(Whiteboard: [sg:want])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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.)
(Reporter)

Comment 1

8 years ago
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.
blocking2.0: --- → ?
Assignee: nobody → Olli.Pettay
Not going to hold the release for this, but I'd approve a patch for this any time.
blocking2.0: ? → -
Patch coming.
Created attachment 531922 [details] [diff] [review]
patch

This is anyway the patch, but cann't be used reliably because of that
nsXPConnect thing.
Ben, why exactly is the NS_RUNTIMEABORT needed there?
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.
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?
(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?
In nsCycleCollector::BeginCollection, which invokes nsXPConnect::Collect(), which invokes JS_GC.

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp#2565
But that's only for "synchronous" cycle collections, which shouldn't be happening?
(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.
Right, but if the GC hasn't been run at all, shouldn't mFirstCollection be true, and thus aForceGC will be set to true?
Ah, yes. Smaug, are you actually hitting the abort?
Yes, I did hit the abort when I was running the chrome test.
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. ;)
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.
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.
Depends on: 663532
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.
Testing if this works now...
Created attachment 539506 [details] [diff] [review]
patch

Seems to work
Attachment #531922 - Attachment is obsolete: true
Attachment #539506 - Flags: review?(gal)
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.
(Reporter)

Updated

8 years ago
Whiteboard: [sg:want]
review ping.
Attachment #539506 - Flags: review?(gal) → review?(jst)

Updated

7 years ago
Attachment #539506 - Flags: review?(jst) → review+
http://hg.mozilla.org/mozilla-central/rev/121904a67387
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Blocks: 677358
Can anyone please provide some guidelines to verify this fix? Is it necessary to create any specific environment to test it?

Thanks!
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.
Considering comment26 setting resolution to VERIFIED-FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.