Closed Bug 707842 Opened 9 years ago Closed 9 years ago
Multiple shapes are sharing Base
Shapes that have Property Tables, which shouldn't happen
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.
9 years ago
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.