636438 - Exact tracing Dictionary causes crashes in smokes for dictionary subclass tests

Status: RESOLVED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P2)

Description

[Tommy Reilly]

FAILURES:
  as3/ShellClasses/DictionarySubclass.abc : as3/ShellClasses/DictionarySubclass.abc  FAILED! diff
  as3/ShellClasses/DictionarySubclass.abc : unexpected exit code expected:0 actual:1 Signal Name: SIGHUP FAILED!
  as3/ShellClasses/DictionarySubclass.abc : captured output: as3/ShellClasses/DictionarySubclass.abc  FAILED! diff|--- ['c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell.exe -Dnocse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell.exe -Dnosse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_s.exe -Dnocse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_s.exe -Dnosse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_sd.exe -Dnocse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_sd.exe -Dnosse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_s.exe -Dnogc']||+++ ['c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_d.exe -Dnocse', 'c:/buildbot/tamarin-redux/windows/builds/5977-5d7dab9731da/windows/avmshell_d.exe -Dnosse']||@@ -1,24 +1,4 @@||DictionarySubclass test non-dynamic subclass of Dictionary class|-SealedDictionary constructor no args = true P***ED!|-SealedDictionary constructor weakKeys=false = true P***ED!|-SealedDictionary constructor weakKeys=true = true P***ED!|-empty SealedDictionary toString = [object SealedDictionary] P***ED!|-SealedDictionary key lookup fails = Error #1069 P***ED!|-weak SealedDictionary key lookup fails = Error #1069 P***ED!|-SealedDictionary key is literal = Error #1056 P***ED!|-weak SealedDictionary key is literal = Error #1056 P***ED!|-SealedDictionary key is object = obj1_value P***ED!|-weak SealedDictionary key is object = obj1_value P***ED!|-literal in SealedDictionary key = false P***ED!|-object in SealedDictionary key = true P***ED!|-literal in weak SealedDictionary key = false P***ED!|-object in weak SealedDictionary key = true P***ED!|-for in SealedDictionary = obj1 P***ED!|-for in weak SealedDictionary = obj1 P***ED!|-for each in SealedDictionary = obj1_value P***ED!|-for each in weak SealedDictionary = obj1_value P***ED!|-delete literal key from SealedDictionary = false P***ED!|-delete object key from SealedDictionary = true P***ED!|-delete literal key from weak SealedDictionary = false P***ED!|-delete object key from weak SealedDictionary = true P***ED!|-||+avmplus crash: exception 0xC0000005 occurred|+Writing minidump crash log to avmplusCrash.dmp|+SIG1||
END FAILURES

Comment 1

[Tommy Reilly]

From ScriptObject::gcTrace
        if (traits()->needsHashtable())

This isn't true for Dictionaries

Comment 2

[Tommy Reilly]

Attachment: trace dictionary hashtable independent of inline ht

Comment 3

[Steven Johnson]

(In reply to comment #1)
> From ScriptObject::gcTrace
>         if (traits()->needsHashtable())

It's possible (but not useful) to make a sealed subclass of Dictionary; there should be an acceptance test for it

Comment 4

[Lars T Hansen]

Comment on attachment 514955 [details] [diff] [review]
trace dictionary hashtable independent of inline ht

I'll have to trust you on the analysis.  Do you mind running it by Steven if you haven't already done so?

Comment 5

[Tommy Reilly]

(In reply to comment #3)
> It's possible (but not useful) to make a sealed subclass of Dictionary; there
> should be an acceptance test for it

Yeah that acceptance test is the one that was crashing when Dictionaries were exactly traced.   Are you pointing this out for info purposes or does it have some bearing on the correctness of the fix?   I couldn't tell.

Comment 6

[Tommy Reilly]

we could rip this out of ScriptObject::gcTrace and have Dictionary override hande this in a hook, I think I like that better...

Comment 7

[Tommy Reilly]

Attachment: better fix

Comment 8

[Lars T Hansen]

Comment on attachment 515098 [details] [diff] [review]
better fix

Agree that makes more sense.

Comment 9

[Tamarin Bot]

changeset: 5986:8c335cdee82e
user:      Tommy Reilly <[e-mail]>
summary:   Bug 636438 - fix exact tracing for Dictionary (r=lhansen)

http://hg.mozilla.org/tamarin-redux/rev/8c335cdee82e

Comment 10

[Tamarin Bot]

changeset: 5987:4bdf638db180
user:      Tommy Reilly <[e-mail]>
summary:   Bug 636438 - make GC_AS3_EXACT tracing not require create method (r=stejohns,sr=lhansen)

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

Comment 11

[Steven Johnson]

Comment on attachment 515098 [details] [diff] [review]
better fix

R- because I don't think this will work for Weak Dictionaries? Or am I confused?