RFR: JDK-8227831: Avoid using volatile for write-once, read-many class field
Claes Redestad
claes.redestad at oracle.com
Wed Jul 17 15:04:45 UTC 2019
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.
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.
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