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

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: njn, Assigned: bhackett)

Tracking

unspecified
mozilla11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Blocks: 637931
No longer depends on: 637931
(Assignee)

Comment 1

6 years ago
Created attachment 579762 [details] [diff] [review]
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.
Attachment #579762 - Flags: review?(luke)

Comment 2

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

Mmm types.
Attachment #579762 - Flags: review?(luke) → review+
(Assignee)

Comment 3

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4a742eac3ab
(Reporter)

Comment 4

5 years ago
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?
(Assignee)

Comment 5

5 years ago
(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

5 years ago
https://hg.mozilla.org/mozilla-central/rev/a4a742eac3ab
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.