Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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
6 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

6 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

6 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

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