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

[Vitaly Davidovich]

Ok Chris, sounds good.

It could be, but I omitted it as it requires a pesky explicit assignment of
> false in the case where there are not final fields!


You could use a local (non-final) boolean to track this state (assigned
with false), and then set the field with it after the looping is over.
Anyway, your call.

On Mon, Feb 23, 2015 at 10:32 AM, Chris Hegarty <chris.hegarty at oracle.com>
wrote:

> On 23/02/15 14:08, Vitaly Davidovich wrote:
>
>> Likewise, seems fine.
>>
>
> Thanks Vitaly.
>
>   By the way, is there a reason not to call
>> freeze() right before returning obj? Is there something special about
>> the place it's invoked at now?
>>
>
> Probably not. The freeze should probably happen before the
> ObjectInputValidation callback, as this justs opens another opportunity for
> early publication of the object, but probably after the handle update and
> check.
>
>  Also, hasFinal field can be final, unless I missed some context in the
>> webrev.
>>
>
> It could be, but I omitted it as it requires a pesky explicit assignment
> of false in the case where there are not final fields!
>
> For completeness, updated webrev in-place, which I intend to push, unless
> there are further comments:
>   http://cr.openjdk.java.net/~chegar/deserialFence/webrev.02/webrev/
>
> -Chris.
>
>  sent from my phone
>>
>> On Feb 23, 2015 7:30 AM, "David Holmes" <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>> 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/
>>
>>         <http://cr.openjdk.java.net/~chegar/deserialFence/webrev.
>> 02/webrev/>
>>
>>
>>     It seems reasonable but I don't have a clear picture of the
>>     connection between readObject and readSerialData.
>>
>>     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.
>>
>>