RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

[David Holmes]

On 20/02/2015 10:13 AM, Vitaly Davidovich wrote:
> In addition to Peter's comment, full fence seems unnecessarily strong and
> will cause performance issues (especially if the fence is per object in the
> graph).  A storeFence should be sufficient here, no?

It should be a fence per graph, or perhaps branches thereof, not per object.

But yes a storeFence (horrible terminology :( ) would suffice given that 
the freeze action in constructors is only OrderAccess::storestore(). And 
Unsafe.storeFence() is OrderAccess::release() which is a 
storestore|storeload barrier.

David

>
> sent from my phone
> On Feb 19, 2015 11:32 AM, "Chris Hegarty" <chris.hegarty at oracle.com> wrote:
>
>> Additional note ( forgotten from original mail):
>>
>> The fence is needed for "final-freeze" is a one-off barrier at the end of
>> deserialization, similar that of constructors . Under normal circumstances
>> the object being deserialized is not visible until deserialization
>> completes, so you don't need a "freeze" until deserialization completes.
>>
>> -Chris.
>>
>> On 19 Feb 2015, at 16:25, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>>
>>> A number of years ago there was a proposal to use Unsafe.put*Volatile
>> methods to set final fields during default deserialisation [1][2], but it
>> never made it due to concerns about the potential negative impact of the
>> additional fences. Now we have a, private, fences API in the platform, I
>> think it is time to revisit this.
>>>
>>> Webrev:
>>>   http://cr.openjdk.java.net/~chegar/deserialFence/webrev.00/webrev/
>>>
>>> Note:
>>>   - Section 17.5.3 in the JLS [3], “Freezes of a final field occur both
>>>     at the end of the constructor in which the final field is set, and
>>>     immediately after each modification of a final field via reflection
>>>     or other special mechanism.” I believe this is a consequence of
>>>     the way in which setting of final fields is supported in the public
>>>     API, Field.setAccessible(), ( as defined by JSR 133 ) and should
>>>     not restrict an implementation from using a more performant
>>>     means, as is suggested here.  The statement in the JLS should
>>>     be revisited.
>>>
>>> - Default Serialization already has a dependency on Unsafe, and
>>>    I don’t see this additional dependency as making that any worse.
>>>
>>> - Open question, should we include volatile fields as well as final?
>>>
>>> - The changes in the webrev will issue a fence even if custom
>>>    deserialization is performed. I think this is ok, as it will be more
>>>    consuming to try to determine if a custom readObject set a final
>>>    through reflection, or not.
>>>
>>> -Chris.
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-6647361
>>> [2]
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-November/005292.html
>>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-December/005456.html
>>> [3]
>> http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.3
>>
>>