1409466 - CC logging seems to be fairly broken on try

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

I enabled CC logging by making this patch: https://hg.mozilla.org/try/rev/8f78b619fe0fcb0c6bb000cc828ed9971a394643, and pushed it to try - It appears to be nearly completely broken based on how many tests went orange with failures like:

Assertion failure: !JS::CurrentThreadIsHeapCycleCollecting(), at /builds/worker/workspace/build/src/js/src/jsgc.cpp:8926 [log…] 

(try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a1e154271d4677c615d08468c2be3ecdcb2541e)

Comment 1

[Andrew McCreight [:mccr8]]

That is strange. Just as a note, logging every CC in a mochitest run is going to produce an absurd amount of logs. I'm not how many files our infrastructure can handle. Even doing it locally for something that takes like 30 seconds to run generates a huge number of logs.

Comment 2

[Andrew McCreight [:mccr8]]

Sorry I've been ignoring this. Hopefully you haven't been too blocked on it.

Jon, do you know what this assertion means? It kind of sounds like we're CCing at some point when we shouldn't be.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1409466-cc-logging

The problem is that nsJSScriptTimeoutHandler calls js::UncheckedUnwrap during cycle collection if CC logging is enabled, and that ends up asserting that we're not inside CC.  This is a good thing, because UncheckedUnwrap can do an expose on the unwrapped object which can alter the result of cycle collection.

We can replace this with a call to js::UncheckedUnwrapWithoutExpose since the result does not escape.

Try build with this change has tests failing with timeouts rather than assertion failures:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=28582d1fc54603c099419f049179dfc16cc92820&group_state=expanded&selectedJob=141963960

Comment 4

[Andrew McCreight [:mccr8]]

Cool, thanks! And yeah, I'd expect timeouts if all CCs are being logged with full logs. ;)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7135395eacd6
Fix assertion failures when CC logging is enabled r=mccr8

Comment 6

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/7135395eacd6