936728 - Call ClearWeakReferences() during unlinking for cycle collected nsSupportsWeakReference classes

Status: RESOLVED DUPLICATE of bug 1535617

(Core :: Cycle Collector, defect)

Description

[Andrew McCreight [:mccr8]]

bz figured out this happens some times in bug 933447.  An unlinked object won't go away immediately, so potentially something can use a weak reference to the object and see the unlinked object, which would be bad.  This probably isn't too terrible nowadays, but it is better to be safe.

One way to do this would be to create a templated helper thing that turns into ClearWeakReferences() for classes that are a subclass of nsSupportsWeakReference, and nothing otherwise, and somehow shove that into the UNLINK boilerplate.  Then nobody can forget to do this.

Comment 1

[Andrew McCreight [:mccr8]]

(In reply to Boris Zbarsky [:bz] from comment #12)
> So Olli had some thoughts on IRC about making do_QueryReferent return null
> for referents with a refcount of 0, I think.  That would provide a less
> whack-a-molish fix for the problem.

Comment 2

[Andrew McCreight [:mccr8]]

I kind of like the idea of doing ClearWeakReference in Unlink, because then we don't have to worry about incomplete unlinking giving an object a refcount of > 0.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, the idea of comment 0 should be pretty simple to do.  The one exception is FragmentOrElement subclasses, which don't use nsSupportsWeakReference.  But we can explicitly clear stuff out in FragmentOrElement's unlink; wouldn't be the first (or even fifth) weird custom thing in there.  ;)

Comment 4

[Boris Zbarsky [:bzbarsky]]

In particular, templating this stuff using IsBaseOf should be quite trivial.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Andrew: still relevant?

Comment 6

[Andrew McCreight [:mccr8]]

Yeah, probably. I don't think it really needs to be a hidden bug, though, as I've never seen any evidence that this happens.