706281 - Cycle collector unlinks nsDOMStringMap twice with weak map, dataset

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Jesse Ruderman]

Attachment: testcase (causes CC crash after closed)

1. Load the testcase
2. Quit Firefox

or

1. Load the testcase
2. Close the testcase
3. Force cycle collection once or twice

Result: Crash [@ nsGenericHTMLElement::ClearDataset]

This might be a regression from bug 668855.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Andrew McCreight [:mccr8]]

Doesn't reproduce for me with 80675:5fd08d019d7d which is from a day before bug 668855 landed.  I'll update and try again.

Comment 3

[Andrew McCreight [:mccr8]]

This actually looks like a dupe of bug 669903, though they didn't have a testcase for it.

Comment 4

[Andrew McCreight [:mccr8]]

And they have a patch, but it looks like they didn't really understand why a null check was needed.  I'll look into this.

Comment 6

[Andrew McCreight [:mccr8]]

Hmm.  Maybe I should undup this.  There's two problems in your test.  The first is that the double unlink crashes, the second is that there's a double unlink at all.  We can leave the other bug as a fix for the former.  I'll look into the latter.

Comment 7

[Andrew McCreight [:mccr8]]

So, like I said in the other patch, double unlinks will happen any time an object is wrapped, and there are two cycle collections in a row without a GC in between.

I think my WeakMap patch is just causing wrapped natives to not be released properly, so this can be hit even if the CC doesn't run twice in a row.  I'm not really sure why that is, but bug 680937 seems to fix this, in that it makes wrapped natives actually be released in a timely fashion.

More broadly, I'm wondering how we can test for double unlinks that don't survive being done twice in a row. I filed bug 708480 for that.

Comment 8

[Andrew McCreight [:mccr8]]

I think this is probably okay.  Wrapped natives just get unlinked twice.  Weak maps may just make it easier to trigger these.