Closed Bug 1367496 Opened 3 years ago Closed 3 years ago

Add some more release assert checks of mScanInProgress

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

Details

Attachments

(1 file)

In bug 1322536, I checked for reentrance in the CC in a few places, but I noticed that there are a few more phases we could check. There's no check at all in CleanupAfterCollection, and we might as well also check in BeginCollection at the top.
Comment on attachment 8870923 [details]
Bug 1367496 - Add more release asserts to the cycle collector.

https://reviewboard.mozilla.org/r/142494/#review146184
Attachment #8870923 - Flags: review?(bugs) → review+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e23de29b6cc
Add more release asserts to the cycle collector. r=smaug
https://hg.mozilla.org/mozilla-central/rev/6e23de29b6cc
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I should check to see if we are hitting these asserts, in a day or two. If not, I'll uplift to beta.
Flags: needinfo?(continuation)
Comment on attachment 8870923 [details]
Bug 1367496 - Add more release asserts to the cycle collector.

Approval Request Comment
[Feature/Bug causing the regression]: old
[User impact if declined]: This is needed to help diagnose crashes we're seeing mostly on beta and release.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: This has been on Nightly for a few days, and we haven't hit the crash.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: If we hit the crash this patch adds, we're probably going to crash in another less safe way later anyways.
[String changes made/needed]: none
Flags: needinfo?(continuation)
Attachment #8870923 - Flags: approval-mozilla-beta?
Next week is RC week for 54, I'm a bit wary of taking this now rather than letting it ride to beta55 in a couple of weeks, especially since this is an old bug.  Any strong reasons for taking it now anyway?
Flags: needinfo?(continuation)
Comment on attachment 8870923 [details]
Bug 1367496 - Add more release asserts to the cycle collector.

No, that's fine.
Flags: needinfo?(continuation)
Attachment #8870923 - Flags: approval-mozilla-beta?
You need to log in before you can comment on or make changes to this bug.