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

[Man Cao]

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/3623fbe2/attachment.html>