Fw: Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

Fri Mar 20 13:03:49 UTC 2015

On 03/20/2015 05:49 AM, Peter Firmstone wrote:
> ----- Original message -----
>  From Peter Levart <peter.levart at gmail.com>
> Sent   Fri, 20 Mar 2015, 01:21:17 EST
> To Peter Firmstone <peter.firmstone at zeus.net.au>, core-libs-dev at openjdk.java.net
> Subject Re: RFR [9] 8071472: Add field access to support setting final fields in readObject
>
>> On 03/19/2015 11:35 AM, Peter Firmstone wrote:
>>> Chris / Peter,
>>>
>>> Perhaps you could consider passing GetFields as a parameter to a
>>> static method (identified by an annotation) and use fieldSetter to
>>> change the fields before they're written?
>>
>> Or change the fields *as* they are written. It actually doesn't matter
>> if the method is static.
>>
>>>
>>> That way it would be possible to not only avoid implementing
>>> readObject or writeObject, but to check invariants and be final field
>>> friendly.
>>>
>>> Just a thought.
>>>
>>> Peter.
>>
>> So you mean something like this or a variant of it. Instead of
>> readObject() instance method, a static method with an additional
>> FieldAccess parameter:
>>
>> @ReadObject
>> private static void altReadObject(ObjectInputStream in, FieldAccess
>> fieldAccess) throws IOException, ClassNotFoundException {
>>             // the same as in readObject, but doesn't have direct access to
>> instance state, so everything must go through FieldAccess API?
>> }
>>
>>
>
> Yes.

An interesting aspect of this approach is that it deals with a problem 
in the serialization spec [1] where it specifically says that 
serializable classes should be reading/writing stream fields always, and 
before reading/writing other data:

In section 3.4: "Either ObjectInputStream's defaultReadObject or 
readFields method must be called once (and only once) before reading any 
optional data written by the corresponding writeObject method; even if 
no optional data is read, defaultReadObject or readFields must still be 
invoked once."

In section 2.3: "Either ObjectOutputStream's defaultWriteObject or 
writeFields method must be called once (and only once) before writing 
any optional data that will be needed by the corresponding readObject 
method to restore the state of the object; even if no optional data is 
written, defaultWriteObject or writeFields must still be invoked once."

But classes (even JDK classes) often disregard this requirement, relying 
on known implementation behavior and either reading/writing optional 
data before fields or just not reading/writing fields at all.  So either 
the spec should be updated (I've tried to do this but nobody seems to 
know how to modify this old content I guess) to match behavior, or the 
spec should be enforced more strictly - however doing the latter *will* 
break a lot of user code, *unless* an alternative readObject method is 
introduced with the more strict enforcement.  But I guess even in this 
case, the spec should be updated to allow the implementation behavior.

[1] 
http://docs.oracle.com/javase/8/docs/platform/serialization/spec/serialTOC.html

>> That's interesting. When operating with FieldSetter API or when calling
>> ObjectInputStream methods, one actually doesn't need an instance
>> context, since it is hidden behind the ObjectInputStream/FieldAccess
>> objects, so the @ReadObject method could be static, which would prevent
>> undesirable direct access to fields (finals in particular).
>>
>> One thing you couldn't do this way is to check the invariants that
>> involve superclass' state that has already been deserialized.
>>
>> But if you don't need that, this could be an alrernative readObject()
>> method. Very easy to implement, actually.
>>
>>
>> Regards, Peter
>>
>
> If you take it one step further, provided the superclass isn't abstract, the api can create a superclass instance that provides api access to superclass state, allowing intraclass invariants to be checked too.
>
> This provides the same benefits as a constructor, without the implementation effort.
>
> Peter.
>
>>>
>>>
>>>> ------------------------------
>>>>
>>>> Message: 5
>>>> Date: Wed, 18 Mar 2015 11:06:56 +0000
>>>> From: Chris Hegarty<chris.hegarty at oracle.com>
>>>> To: Alan Bateman<Alan.Bateman at oracle.com>,       Peter Levart
>>>> <peter.levart at gmail.com>
>>>> Cc: Core-Libs-Dev Libs<core-libs-dev at openjdk.java.net>
>>>> Subject: Re: RFR [9] 8071472: Add field access to support setting
>>>> final fields       in readObject
>>>> Message-ID:<55095C50.3010605 at oracle.com>
>>>> Content-Type: text/plain; charset=windows-1252; format=flowed
>>>>
>>>> On 17/03/15 13:42, Alan Bateman wrote:
>>>>> On 17/03/2015 12:21, Peter Levart wrote:
>>>>>> Hi Alan,
>>>>>>
>>>>>> I agree that not calling defaultReadObject() from readObject() and
>>>>>> having a final field is potentially a bug. But need not be in case
>>>>>> some other means of setting final fields was used (Unsafe or
>>>>>> reflection). Some readObject() implementations in base module that
>>>>>> Chris changed to use new API fall into this category. We can't track
>>>>>> those usages, so to keep backwards compatibility, this checking has to
>>>>>> be opt-in. Is there a more elegant way to opt-in? A
>>>>>> @CheckFinalsAssignment annotation on the readObject() method?
>>>>> I'm not sure that an annotation is right here. Instead then it might
>>>>> work as a method on FieldSetter to enable strict checking.
>>>> Peter suggested a method on FieldSetter to enable strict checking before
>>>> too, and at the time I pushed back. After this discussion, I've come
>>>> full circle, and maybe this opt-in style method is a reasonable
>>>> compromise. Something like:
>>>>
>>>> /**
>>>> * Checks that all final instance fields, declared by the class where
>>>> * the {@code readObject} callback is being invoked, have been set.
>>>> *
>>>> *<p>   This method is intended to be called as a final step after all
>>>> * final instance fields have been set.
>>>> *
>>>> * @throws InvalidObjectException if one, or more, final instance
>>>> *                 fields have not been set
>>>> */
>>>> void checkAllFinalsSet() throws InvalidObjectException;
>>>>
>>>>
>>>> ...and the user code would look similar to this, from java.io.File:
>>>>
>>>> s.fieldSetter().set("path", p)
>>>> .set("prefixLength", pLen)
>>>> .checkAllFinalsSet();
>>>>
>>>> -Chris.
>>>>
>>>>
>>>>
>>>>
>>>> End of core-libs-dev Digest, Vol 95, Issue 54
>>>> *********************************************
>>>
>>
>

-- 
- DML