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 <m...@google.com
<mailto:m...@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.hol...@oracle.com
<mailto:david.hol...@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
