If you forget to add a create function when using GC_AS3_EXACT you crash assert in hard to understand ways

RESOLVED FIXED in Q3 11 - Serrano

Status

Tamarin
Garbage Collection (mmGC)
P1
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Tommy Reilly, Assigned: Tommy Reilly)

Tracking

unspecified
Q3 11 - Serrano
Dependency tree / graph

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
We should either:

1) make them add it by not compiling somehow
2) provide it for them 
3) invoke new directly since the generated createInstanceProc will be the only caller and that's what the non-exact case does.
(Assignee)

Comment 1

7 years ago
For now I've just used macros to get these into the code with little typing, then I don't feel like I've undone the work Steven did to remove the createInstance methods.   I think we can do better.

Comment 2

7 years ago
I could live with any of these; IMHO (3) is best since AS3 objects should (generally) only be created via the class constructor anyway.
(Assignee)

Comment 3

7 years ago
So #3 is a simple tweak to nativegen.py, but then I have to remove all the create methods in tamarin that are otherwise not used, Lar's you down with this plan?

Comment 4

7 years ago
Would be interested to know what is in the "otherwise not used" category... hopefully nothing.
(Assignee)

Comment 5

7 years ago
I meant in tamarin core, we have custom create functions for a bunch of different core builtins there.
(Assignee)

Comment 6

7 years ago
Created attachment 514951 [details] [diff] [review]
Make things better

- you can never forget MMgc::kExact now
- still have protected ctors' but objects have to friend the class
- all create calls never called from non-generated code are gone

All in all should be more foolproof for our users.
Attachment #514951 - Flags: superreview?(lhansen)
Attachment #514951 - Flags: review?(stejohns)
(Assignee)

Comment 7

7 years ago
note b/c this forces MMgc::kExact this patch is blocked by the dictionary issue
Depends on: 636438

Updated

7 years ago
Attachment #514951 - Flags: review?(stejohns) → review+

Comment 8

7 years ago
Comment on attachment 514951 [details] [diff] [review]
Make things better

I don't understand what "hosed" means in the title.  Does it not compile?  Does it crash?  What happens?

Technically the patch seems fine.

In terms of usability I'm not 100% sure it's a good idea because it creates yet another special case for the user: you should write a create method /except/ if it's a glue class in which case it will be generated for you...

(Not taking a position on the review until the bug's been clarified further.)

Updated

7 years ago
Blocks: 636703
(Assignee)

Comment 9

7 years ago
(In reply to comment #8)
> I don't understand what "hosed" means in the title.  Does it not compile?  Does
> it crash?  What happens?

It means you end calling some C++ ancestors create method instead of yours and your object ends up smaller than you need, your ctor didn't run and you usually get overwrite asserts.

> In terms of usability I'm not 100% sure it's a good idea because it creates yet
> another special case for the user: you should write a create method /except/ if
> it's a glue class in which case it will be generated for you...

No I think the new rule is you never write a create method unless you want to.   For GC_AS3_EXACT the generated will invoke new directly so don't bother with a create method unless your objects are created from C++ (doesn't happen except in the tamarin core).   Really I think the use of a create method is just a "best practice" for GC_CPP_EXACT classes.
(Assignee)

Updated

7 years ago
Summary: If you forget to add a create function when using GC_AS3_EXACT you get hosed → If you forget to add a create function when using GC_AS3_EXACT you crash assert in hard to understand ways

Comment 10

7 years ago
Comment on attachment 514951 [details] [diff] [review]
Make things better

I get it.
Attachment #514951 - Flags: superreview?(lhansen) → superreview+
(Assignee)

Comment 11

7 years ago
changeset: 5987:4bdf638db180
user:      Tommy Reilly <treilly@adobe.com>
summary:   Bug 636438 - make GC_AS3_EXACT tracing not require create method
(r=stejohns,sr=lhansen)

http://hg.mozilla.org/tamarin-redux/rev/4bdf638db180
(Assignee)

Comment 12

7 years ago
I screwed the bug #'s but there she is
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.