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

[Chris Hegarty]

On 23/02/15 12:30, David Holmes wrote:
> Hi Chris,
>
> On 23/02/2015 9:01 PM, Chris Hegarty wrote:
>> Peter, David, Vitaly,
>>
>> Can you please take a look at the latest version of this change:
>>
>>    http://cr.openjdk.java.net/~chegar/deserialFence/webrev.02/webrev/
>
> It seems reasonable

Thanks.

> but I don't have a clear picture of the connection
> between readObject and readSerialData.

readObject() is the public entry point to re-constructing an object from 
the stream. readSerialData is responsible for reading, from the stream, 
and setting the objects field values. readSerialData is currently the 
only way, without the use of reflection and a custom 
readObject(ObjectInputStream), that the Serialization implementation can 
set final fields.

Thanks for your review and comments.

-Chris.

> David
>
>> On 20/02/15 15:09, Peter Levart wrote:
>>> ...
>>> This looks good now. But I wonder if issuing fences after nested calls
>>> to readObject() makes much sense. If cycles are present in a subgraph
>>> deserialized by nested call to readObject(), a field pointing back to an
>>> object not part of the subgraph stream will point to the object that
>>> will not be fully initialized yet, so nested calls to readObject()
>>> should not be expected to return a reference to a fully constructed
>>> subgraph anyway. Only top-level call is guaranteed to return a fully
>>> constructed graph.
>>
>> Right. I was never convinced of this myself either. Removed. Unnecessary
>> complication.
>>
>>> If you optimize this and only issue one fence for top-level call to
>>> readObject(), tracking of 'requiresFence' (I would call it
>>> requiresFreeze to be in line with JMM terminology - the fence is just a
>>
>> 'requiresFreeze' is better. Updated
>>
>>> way to achieve freeze) can also be micro-optimized. You currently do it
>>> like this:
>>>
>>> 1900             requiresFence |= slotDesc.hasFinalField();
>>>
>>> which is a shortcut for:
>>>
>>> requiresFence = requiresFence | slotDesc.hasFinalField();
>>>
>>> ...which means that the field is overwritten multiple times
>>> unnecessarily and slotDesc.hasFinalField() is called every time. You can
>>> write the same logic as:
>>>
>>> if (!requiresFence && slotDesc.hasFinalField()) {
>>>      requiresFence = true;
>>> }
>>
>> ... and it is more readable. Updated.
>>
>>> There will be at most one write to the field and potentially less calls
>>> to slotDesc.hasFinalField().
>>
>> -Chris.