StMNHTIterator and StTraitsBindingsIterator need some volatile love

VERIFIED FIXED

Status

VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: stejohns, Unassigned)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
Created attachment 395729 [details] [diff] [review]
Patch

StMNHTIterator needs to keep a reference to the MNHT it is using to keep it from being collected.

StMNHTIterator and StTraitsBindingsIterator both need their "keeper" fields to be marked volatile; turns out that agressive optimizing compilers can notice that these fields aren't ever used and omit the store.
Attachment #395729 - Flags: review?(treilly)
(Reporter)

Comment 1

9 years ago
Created attachment 396868 [details] [diff] [review]
StringIndexer, too

StringIndexer needs similar love, too.
Attachment #396868 - Flags: review?(treilly)

Updated

9 years ago
Attachment #395729 - Flags: review?(treilly) → review+

Comment 2

9 years ago
Should we have a all our stack utility classes w/ GC pointers be marked volatile?

Updated

9 years ago
Attachment #396868 - Flags: review?(treilly) → review+
(Reporter)

Comment 3

9 years ago
(In reply to comment #2)
> Should we have a all our stack utility classes w/ GC pointers be marked
> volatile?

Based on what we saw here, yes, potentially. (The issue here was that the "keeper" fields weren't used for anything, so the compiler optimized 'em away. If a GC pointer is actually *used* then the volatile probably isn't necessary.
(Reporter)

Comment 4

9 years ago
pushed both to redux as changeset:   2418:8b3c7bde9273
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 5

9 years ago
Resolved fixed engineering / work item that has been pushed.  Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.