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

[Chris Hegarty]

On 20/02/15 00:27, David Holmes wrote:
> 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.

My original intent was to replicate the final-freeze action as performed 
by constructors. What I am hearing is that we do better, without any 
visible side-effect. Since the graph is not published until 
readObject/Unshared returns, the fence can be added there.

That said, it may be possible for one leaf in the graph to reference 
another, but to observe this ( in readObject ), there would have an 
implicit dependency on re-constructor order, which is fragile, at best.

> 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.

Thanks, updated.

Updated Webrev:
   http://cr.openjdk.java.net/~chegar/deserialFence/webrev.01/webrev/

Note, the changes in this webrev are overly defensive in the face of 
recursive calls to readObject/Unshared. This should be ok, but probably 
not strictly necessary.

-Chris.


> 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
>>>
>>>