Need selftest for WeakValueHashtable

VERIFIED FIXED in Q3 11 - Serrano

Status

Tamarin
Garbage Collection (mmGC)
P3
normal
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Brent Baker, Assigned: Lars T Hansen)

Tracking

unspecified
Q3 11 - Serrano
Bug Flags:
in-testsuite +
flashplayer-bug -
flashplayer-triage +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Currently there is no usage in the shell for WeakValueHashtable so we will need to create a selftest that uses this object. Create selftest so that we can get code coverage of this via the shell.
Flags: in-testsuite?
Flags: flashplayer-triage+
Flags: flashplayer-bug-
(Assignee)

Comment 1

7 years ago
Created attachment 526352 [details] [diff] [review]
Patch
Attachment #526352 - Flags: review?(brbaker)
(Assignee)

Comment 2

7 years ago
Sorry, not a patch - just a file.

Comment 3

7 years ago
changeset: 6188:3070ff6dee1f
user:      Brent Baker <brbaker@adobe.com>
summary:   Bug 650367: add selftest for WeakValueHashtable (p=lhansen,r=rulohani,brbaker)

http://hg.mozilla.org/tamarin-redux/rev/3070ff6dee1f
(Reporter)

Updated

7 years ago
Attachment #526352 - Flags: review?(brbaker) → review+
(Reporter)

Updated

7 years ago
Assignee: nobody → lhansen
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED

Comment 4

7 years ago
The selftest is failing on linux-arm:

['start', 'avmplus', 'builtins']
['test', 'avmplus', 'builtins', 'WeakValueHashTable']
['failure', 'avmplus', 'builtins', 'WeakValueHashTable', 'sum >= 250 && sum <= 300', 'ST_avmplus_builtins.st', 60]
['end', 'avmplus', 'builtins']

full log: http://tamarin-builds.mozilla.org/tamarin-redux/builders/linux-arm-test/builds/396/steps/Testsuite_Selftest_Release/logs/stdio
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

7 years ago
Yes, there is a weakness in the test: an aggressive compiler can clear out the stack variable that holds onto the string that should not be collected.  It needs to be anchored more carefully.
(Assignee)

Comment 6

7 years ago
Created attachment 528016 [details] [diff] [review]
Lock the "permanent" object
Attachment #528016 - Flags: review?(cpeyer)
(Assignee)

Updated

7 years ago
Priority: -- → P3
Target Milestone: --- → Q3 11 - Serrano
(Reporter)

Comment 7

7 years ago
Comment on attachment 528016 [details] [diff] [review]
Lock the "permanent" object

I am running this patch through the sandbox now.
(Reporter)

Comment 8

7 years ago
Comment on attachment 528016 [details] [diff] [review]
Lock the "permanent" object

Review of attachment 528016 [details] [diff] [review]:

(In reply to comment #7)
> Comment on attachment 528016 [details] [diff] [review]
> Lock the "permanent" object
> 
> I am running this patch through the sandbox now.

Selftest is still failing on linux-arm
Attachment #528016 - Flags: review-
(Assignee)

Comment 9

7 years ago
In that case you need to modify the test to print out the actual value of "sum", since I don't understand what's going on.
(Reporter)

Comment 10

7 years ago
(In reply to comment #9)
> In that case you need to modify the test to print out the actual value of
> "sum", since I don't understand what's going on.

On linux arm, with the patch to lock the object (attachment #528016 [details] [diff] [review]), the sum is 0 at the end of the loop

ST_avmplus_builtins.st
AvmLog("sum=%d\n",sum);
// Retain at least 250, but it would be unreasonable to retain more than 300
%%verify sum >= 250 && sum <= 300

selftest output
['test', 'avmplus', 'builtins', 'WeakValueHashTable']
sum=0
['failure', 'avmplus', 'builtins', 'WeakValueHashTable', 'sum >= 250 && sum <= 300', 'ST_avmplus_builtins.st', 63]
(Reporter)

Comment 11

7 years ago
Release build has a sum=0
Debug build has a sum=251

Comment 12

7 years ago
changeset: 6214:eeb68bddb214
user:      Brent Baker <brbaker@adobe.com>
summary:   Bug 616125: add testing around XML.setNotification() (r=treilly)

http://hg.mozilla.org/tamarin-redux/rev/eeb68bddb214
(Reporter)

Comment 13

7 years ago
(In reply to comment #12)
> changeset: 6214:eeb68bddb214
> user:      Brent Baker <brbaker@adobe.com>
> summary:   Bug 616125: add testing around XML.setNotification() (r=treilly)
> 
> http://hg.mozilla.org/tamarin-redux/rev/eeb68bddb214

This is the second line of the commit message, which makes much more sense on the context of this bug:

"Bug 650367: disable WeakValueHashtable() selftest on ARM until issue is resolved"
(Assignee)

Comment 14

7 years ago
(In reply to comment #11)
> Release build has a sum=0
> Debug build has a sum=251

That is incomprehensible to me...
(Assignee)

Updated

7 years ago
Attachment #528016 - Flags: review?(cpeyer)
(Assignee)

Updated

7 years ago
Depends on: 654052
(Assignee)

Comment 15

7 years ago
(In reply to comment #14)
> (In reply to comment #11)
> > Release build has a sum=0
> > Debug build has a sum=251
> 
> That is incomprehensible to me...

No longer.  Bug 654052 logged to deal with a bug in the lock API.

I managed to repro the problem on Mac Desktop release builds, I'll post a new patch here.
(Assignee)

Comment 16

7 years ago
Created attachment 529424 [details] [diff] [review]
Lock the "permanent" object, v2

With this update, we expose on Mac release builds the ARM problem, which is fixed in bug #654052, and that patch is required to test the present patch.
Attachment #526352 - Attachment is obsolete: true
Attachment #528016 - Attachment is obsolete: true
Attachment #529424 - Flags: review?(brbaker)
(Reporter)

Comment 17

7 years ago
Comment on attachment 529424 [details] [diff] [review]
Lock the "permanent" object, v2

Looks good, but please regen the testcase with the "%%ifndef    VMCFG_ARM" removed from the *.st file so that the testcase is run on ARM.

I ran this through the sandbox along with the patch from 654052 and everything ran cleanly.
Attachment #529424 - Flags: review?(brbaker) → review+
(Assignee)

Updated

7 years ago
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED

Comment 18

7 years ago
changeset: 6249:0d858bab2de1
user:      Lars T Hansen <lhansen@adobe.com>
summary:   Fix 650367 - Need selftest for WeakValueHashtable (r=brbaker)

http://hg.mozilla.org/tamarin-redux/rev/0d858bab2de1
(Reporter)

Comment 19

7 years ago
tamarin-redux-serrano brbaker$ hg log -r 380d7261b801
changeset:   6285:380d7261b801
user:        Lars T Hansen <lhansen@adobe.com>
date:        Mon May 02 17:09:58 2011 +0200
summary:     Fix 650367 - Need selftest for WeakValueHashtable (r=brbaker)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.