RFR: JDK-8227831: Avoid using volatile for write-once, read-many class field

David Holmes david.holmes at oracle.com
Thu Jul 18 04:26:09 UTC 2019


Hi Claes,

On 18/07/2019 1:04 am, Claes Redestad wrote:
> Hi,
> 
> removing volatile aligns LangReflectAccess initialization with the
> pattern used for other access objects: it's only initialized in the
> static initializer of some class which we ensure is initialized, which
> means any initialization race is guarded by the initialization of said
> class - in this case j.l.r.Modifier.

If the field is not volatile then we are not guaranteed to correctly see 
the constructed LangReflectAccess instance. Without volatile (or without 
additional use of unconditional sync in the accessor) we do not have a 
happens-before relationship between the construction of the LRA 
instance, and the setting of the field.

> Initialization of other access objects are not guarded by any
> explicit synchronization, however, since similar load/store barriers
> will be in effect by ensuring the class that constructs the object has
> been initialized. So I think you could remove the explicit
> synchronization.

We are not guaranteed to hit the class initialization checks that would 
guarantee the necessary ordering.

David
-----

> I'm not sure why LangReflectAccess has not moved in with other
> SharedSecrets. Perhaps this is just an artefact of having been around
> for a long time, but maybe that'd cause some cyclic bootstrap
> dependency. Either way it's nice to see it align to use the same
> pattern.
> 
> Thanks!
> /Claes
> 
> On 2019-07-17 10:00, Kazunori Ogata wrote:
>> Hi Aleskey,
>>
>> Thank you for your comment.
>>
>> I forgot to mention one thing.  I verified that all accesses to
>> langReflectioAccess calls the accessor "langReflectAccess()".  Since this
>> variable is modified once from null to non-null, I think the write in the
>> setter is guaranteed to be visible in the getter by putting the
>> synchronized block in langReflectAccess().
>>
>> Should I put comments about this assumption?  Actually, in JDK11 there 
>> are
>> some lines that do not call the getter, so backport needs to fix them,
>> too.
>>
>>
>> Regards,
>> Ogata
>>
>>
>> Aleksey Shipilev <shade at redhat.com> wrote on 2019/07/17 16:49:08:
>>
>>> From: Aleksey Shipilev <shade at redhat.com>
>>> To: Kazunori Ogata <OGATAK at jp.ibm.com>, core-libs-dev at openjdk.java.net
>>> Date: 2019/07/17 16:49
>>> Subject: [EXTERNAL] Re: RFR: JDK-8227831: Avoid using volatile for
>> write-
>>> once, read-many class field
>>>
>>> On 7/17/19 8:48 AM, Kazunori Ogata wrote:
>>>> Webrev: http://cr.openjdk.java.net/~ogatak/8227831/webrev/
>>>
>>> Note this is generally not safe: it introduces data race on
>>> langReflectAccess field access. It has
>>> to be proved that everything that implements LangReflectAccess interface
>>
>>> makes this data race
>>> benign: e.g. all fields that are accessed via those implementation are
>>> final, read once, etc. And
>>> briefly looking at that, I am not sure it is the case for the actual
>>> accessor generators.
>>>
>>> I'd cautiously leave "volatile" here.
>>>
>>> -- 
>>> Thanks,
>>> -Aleksey
>>>
>>> [attachment "signature.asc" deleted by Kazunori Ogata/Japan/IBM]
>>


More information about the core-libs-dev mailing list