Closed Bug 1222420 Opened 4 years ago Closed 4 years ago

AutoFinishGC is confusing

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jandem, Assigned: jonco)

Details

Attachments

(1 file)

AutoFinishGC looks like a RAII class, so readers may think it means the destructor will finish the GC, or that we won't GC in its scope.

But it only has a constructor, so we could replace it with a function, js::FinishGC(rt) or something.
I also found this strange.  I guess the reason is to finish the GC before the AutoTraceSession is constructed in AutoPrepareForTracing, but we can do that by wrapping it in a Maybe<>.
Assignee: nobody → jcoppeard
Attachment #8729524 - Flags: review?(terrence)
Comment on attachment 8729524 [details] [diff] [review]
bug1222420-auto-finish-gc

Review of attachment 8729524 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgc.cpp
@@ -6732,5 @@
>          JS::PrepareForIncrementalGC(rt);
>          JS::FinishIncrementalGC(rt, JS::gcreason::API);
>      }
>  
> -    rt->gc.waitBackgroundSweepEnd();

\o/  Figured I'd miss at least one of these.
Attachment #8729524 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/d6ce8d232178
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.