707842 - Multiple shapes are sharing BaseShapes that have PropertyTables, which shouldn't happen

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

DMD tells me that PropertyTables are being counted twice.  The attached file contains the error message.  There's only one place that PropertyTables are counted (the shape->sizeOfPropertyTable() call in XPCJSRuntime.cpp), so the problem doesn't appear to be with the memory reporting code.  

bhackett said on IRC: "base shapes with property tables are supposed to be 1:1 with the shapes that own them, but if multiple shapes have the same owned base shape then the double counting could happen".  So it's probably a problem with the handling of Shapes and BaseShapes.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: improve Shape constructors

Improve the types of the shape constructors to require that an unowned base shape be initially supplied.  Doing this caught one instance we were passing a possibly owned base shape, in JSObject::addPropertyInternal, though I haven't confirmed this is definitely the cause of this bug.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 579762 [details] [diff] [review]
improve Shape constructors

Mmm types.

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4a742eac3ab

Comment 4

[Nicholas Nethercote [inactive]]

I just did a couple of DMD runs and I'm not getting the warnings about double-counting any more!  Good news.

bhackett, could bad things have happened as a result of this bug?

Comment 5

[Brian Hackett [Laid off!]]

(In reply to Nicholas Nethercote [:njn] from comment #4)
> I just did a couple of DMD runs and I'm not getting the warnings about
> double-counting any more!  Good news.
> 
> bhackett, could bad things have happened as a result of this bug?

Sharing an owned base shape between multiple shapes could cause incorrect lookups --- one or the other shape will have an incorrect property table, and when doing a search for a native property of the object a shape might not be found even though it is on the object.  I *think* that is the only fallout of this particular problem.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a4a742eac3ab