RFR(s): 8191859: SoftReference clock/timestamp should be declared volatile
per.liden at oracle.com
Mon Nov 27 10:25:44 UTC 2017
On 2017-11-24 23:56, Martin Buchholz wrote:
> These fields have been this way for a very long time, probably
> intentionally non-volatile.
> Is there an observable problem being solved here?
Sorry for not being more clear about that. Having them non-volatile can
lead to the timestamp going backwards (e.g. in C2, if a clock load is
cached over a safepoint), which in turn can lead to incorrect decision
being made by the GC SoftRef policy about whether the referent should be
cleared or not. While an incorrect decision is sub-optimal, it's not
catastrophic. Worst case is that we clear a soft ref per-maturely.
However, after some discussions here we realized that the suggested fix
isn't enough to prevent this from happening, e.g. when the interpreter
is running without thread local safepoint polling. A robust fix for this
would likely need a CAS to prevent the timestamp from going backwards.
For these reasons, I'm withdrawing my suggested change for now. I need
to think more about the implication of a robust fix, or if we should
just live with this issue.
> I see that out-of-thin-air longs are theoretically possible, but very
> rare in practice on current platforms.
> If all you're trying to do is to ensure atomicity and that the
> reads/writes don't get optimized away by the jit, then consider using
> "opaque" VarHandles instead.
> I was surprised there was no bug subcomponent for java.lang.ref
> On Fri, Nov 24, 2017 at 5:13 AM, Per Liden <per.liden at oracle.com
> <mailto:per.liden at oracle.com>> wrote:
> The clock and timestamp fields in SoftReference should be declared
> volatile. These fields are read or updated by the VM, (possibly)
> concurrently with the execution of Java threads.
> To be more specific, the timestamp field is read by the GC during
> reference discovery, e.g. during G1/CMS concurrent mark. The clock
> is updated by the GC, typically after reference processing has
> Webrev: http://cr.openjdk.java.net/~pliden/8191859/webrev.0/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191859
More information about the core-libs-dev