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

VERIFIED FIXED

Status

()

Core
DOM
--
enhancement
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, 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

7 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

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

Updated

7 years ago
Assignee: nobody → Olli.Pettay
Not going to hold the release for this, but I'd approve a patch for this any time.
blocking2.0: ? → -
(Assignee)

Comment 3

6 years ago
Patch coming.
(Assignee)

Comment 4

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

Comment 5

6 years ago
Created attachment 531922 [details] [diff] [review]
patch

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

Comment 6

6 years ago
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?
(Assignee)

Comment 15

6 years ago
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.
(Assignee)

Comment 20

6 years ago
Testing if this works now...
(Assignee)

Comment 21

6 years ago
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

6 years ago
Whiteboard: [sg:want]
(Assignee)

Comment 23

6 years ago
review ping.
(Assignee)

Updated

6 years ago
Attachment #539506 - Flags: review?(gal) → review?(jst)

Updated

6 years ago
Attachment #539506 - Flags: review?(jst) → review+
(Assignee)

Comment 24

6 years ago
http://hg.mozilla.org/mozilla-central/rev/121904a67387
Status: NEW → RESOLVED
Last Resolved: 6 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!
(Assignee)

Comment 26

6 years ago
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.