On 2017-07-14 15:46, Aleksey Shipilev wrote:
On 07/14/2017 03:40 PM, Claes Redestad wrote:
signature is already made volatile in this patch, deliberately to ensure that
for the private constructors we keep correctness:
in these private constructors, the signature field is initialized in the
constructor (we can assert that it's non-null) to a value that would not
match getClassSignature(type), but since the field is volatile it will never
be observed as null by a call to getSignature() on an object created with
these constructors (unless I'm mistaken, and we *do* need an added
synchonization point here).
Thing is, "volatile" is not as strong as "final" for racy initializations:
https://shipilev.net/blog/2016/close-encounters-of-jmm-kind/#wishful-volatiles-are-finals
If that is not a concern -- e.g. if private constructor is never used to make
shared OSFs -- then "volatile" is superfluous. If it is a concern, then
"volatile" is not enough, and you have to keep "final".
Gosh!
Seeing how it's possible to lookup ObjectStreamClass and get at
ObjectStreamField's that might have been created from the internal
constructors, I think it *is* a concern.
Thus the only viable option that allows us to be correct *and* lazy
might be to
add a new field that is lazily initialized only when using the public
constructors:
http://cr.openjdk.java.net/~redestad/8184603/jdk.02/
WDYT? Adding a field is of course a possible footprint increase, but
ObjectStreamField is a rare object in most applications so footprint
risk is
minimal to begin with, and since the signature String will now be
created lazily
this is likely still a net footprint win for many (most?) applications.
Thanks!
/Claes
The hair-pull scenario under the race would be like this: OSF gets constructed
with Ljava/lol/ConcurrencyIsHard, published, another thread calls getSignature()
on it, eads null from $signature, regenerates it from Object $type, returns
Ljava/lang/Object as signature. Something fails not expecting j/l/Object in
signature, you go debug it, and see getSignature() returns
Ljava/lol/ConcurrencyIsHard, as if nothing had happened.
Thanks,
-Aleksey
