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

[Jean Christophe Beyler]

Hi Man,

Not surprisingly, this looks good to me, though I'm not an official
reviewer :),
Jc



On Wed, May 1, 2019 at 2:36 PM David Holmes <david.holmes at oracle.com> wrote:

> On 2/05/2019 6:25 am, Man Cao wrote:
> > I have moved the set_tag_map back to the constructor:
> > https://cr.openjdk.java.net/~manc/8223177/webrev.01/
>
> Looks good - thanks. I doubt there will be any additional use of this
> constructor.
>
> David
> -----
>
> > -Man
> >
> >
> > On Wed, May 1, 2019 at 11:05 AM Man Cao <manc at google.com
> > <mailto: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
> >     <mailto: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
> >
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190501/e524ef7b/attachment-0001.html>