RFR (S): 8223177: Data race on JvmtiEnvBase::_tag_map in double-checked locking

[Man Cao]

I have moved the set_tag_map back to the constructor:
https://cr.openjdk.java.net/~manc/8223177/webrev.01/

-Man


On Wed, May 1, 2019 at 11:05 AM Man Cao <manc at google.com> wrote:

> Thanks for the review.
> I moved set_tag_map out of the constructor because the release store is
> only required in the double-checked locking pattern.
> If the constructor is called in a single-threaded context, or if _tag_map
> is always protected by a lock, then it could use the normal store instead.
> Currently it doesn't matter since the constructor is only called inside
> the double-checked locking.
> I'm OK either way. Do you prefer to keep it inside the constructor?
>
> -Man
>
>
> On Wed, May 1, 2019 at 4:02 AM David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Man,
>>
>> On 1/05/2019 11:51 am, Man Cao wrote:
>> > Hi all,
>> >
>> > Can I have reviews for this small change that adds memory fences for
>> > double-checked locking?
>> > We found this race while working on the Java ThreadSanitizer project.
>> >
>> > Webrev: https://cr.openjdk.java.net/~manc/8223177/webrev.00/
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8223177
>>
>> Looks fine. One query in jvmtiTagMap.cpp - Was there a particular reason
>> you moved the set_tag_map out of the constructor? (It's a common pattern
>> when objects are bi-directionally linked to do it in the constructor.)
>>
>> Thanks,
>> David
>>
>> > -Man
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190501/554108f4/attachment.html>