937826 - ICC: Implement dynamic soundness checking for ICC in debug builds

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

There are two standard correctness properties for a collector: soundness and completeness.  Soundness says that we only collect garbage, and completeness says that we collect all garbage.  We can dynamically check the correctness of incremental cycle collection relative to synchronous cycle collection (which of course is perfect and bug free) by running a non-destructive synchronous CC at the start of a CC, storing the set of objects we think should be collected, then compare that to the set of objects we collect in the incremental CC.  By "non-destructive", I mean that object are not removed from the purple buffer, and CollectWhite isn't run.

Soundness means that all objects incrementally collected are synchronously collected, and completeness means that all objects synchronously collected are also incrementally collected.

That's all well and good, but there are some wrinkles here. First of all, the set of actual garbage objects at the start of an ICC is not the same as at the end of ICC: an object can become garbage during the course of an ICC (false positive for soundness checking), or a weak ref could turn a garbage object into a non-garbage object (false positive for completeness checking).

The latter can happen even if the weak ref is only temporarily converted into a strong ref, because we detect that the object has been AddRefed and treat it as being alive (bug 937818).  Therefore, I'm not going to try to get completeness checking working right now.  My understanding is that IGC only checks soundness, not completeness.

False positives by objects becoming garbage during ICC are mitigated by the mechanisms in bug 937818.  If an object becomes garbage during an ICC, it must get Released, and thus put into the purple buffer, and so through the mechanism in bug 937818, it is being treated as live, preventing the false positive.  Thus my approximation is actually useful.  A similar thing can be used for XPCWN/XPCWJS, by setting the dirty bit in AddRef in addition to Release. I'm not sure I actually do that right now, though.

A more annoying problem with soundness false positives is that an XPCWN decides to trace (or not) its JS object depending on its ref count.  See bug 927601.  I'm not sure exactly how to fix that yet.  I'm trying to remove that mechanism (we'd always traverse the native), which I think is not needed for cycle collected wrapped natives.

I believe that my dynamic correctness checker has only ever had false positives, FWIW.

Comment 1

[Andrew McCreight [:mccr8]]

CC checkingt hasn't yet actually found any errors, so I think it can be delayed to the later polishing-for-enabling-by-default phase of things.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Add support for non-destructive SelectPointers

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: part 2 - implement it, WIP

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Add support for non-destructive SelectPointers

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: part 2 - implement it, WIP

Comment 6

[Andrew McCreight [:mccr8]]

This was a cool idea, but ICC only had one bug in it that we've noticed since landing, and I couldn't get this checking working due to XPConnect annoyingness, so I think I'll just let this go.