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

[Chris Hegarty]

On 23/02/15 15:30, Vitaly Davidovich wrote:
> 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.

That is better. Done.

Thanks,
-Chris.

> On Mon, Feb 23, 2015 at 10:32 AM, Chris Hegarty
> <chris.hegarty at oracle.com <mailto: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/
>     <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>
>         <mailto: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/>
>
>
>         <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.
>
>