Closed Bug 707842 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: njn, Assigned: bhackett)

References

Details

Attachments

(1 file)

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.
Blocks: ObjectShrink
No longer depends on: ObjectShrink
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.
Attachment #579762 - Flags: review?(luke)
Comment on attachment 579762 [details] [diff] [review]
improve Shape constructors

Mmm types.
Attachment #579762 - Flags: review?(luke) → review+
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?
(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.
https://hg.mozilla.org/mozilla-central/rev/a4a742eac3ab
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.