RFR [9] 8071472: Add field setter API for setting final fields in readObject

[Chris Hegarty]

Peter,

On 25/03/15 12:24, Peter Levart wrote:
> On 03/23/2015 04:45 PM, Chris Hegarty wrote:
>> Here is an updated version of the FieldSetter API, with all comments
>> to date incorporated.
>>
>>
>> http://cr.openjdk.java.net/~chegar/8071472/02/specdiff/overview-summary.html
>>
>>
>> Final spec comments welcome, after which I intend to submit a CCC
>> request.
>>
>> -Chris.
>
> Hi Chris,
>
> Recent bug "JDK-8068721 - RMI-IIOP communication fails when
> ConcurrentHashMap is passed to remote method" made me thinking...
>
> ObjectInputStream is an extensible API. There are subclasses of
> ObjectInputStream out there.

I did think about this previously, but thought it ok ( I assumed that 
overridden methods would first invoke their superclass counterparts ). 
It appears that InputStreamHook does not do this.

 > For example, there is an abstract
> "com.sun.corba.se.impl.io.InputStreamHook" with concrete implementation
> "com.sun.corba.se.impl.io.IIOPInputStream". The InputStreamHook
> overrides ObjectInputStream.defaultReadObject() with it's own
> implementation which doesn call ObjectInputStream.defaultReadObject(),
> which means that FieldSetterContext.checkPersistentFinalsNotSet() and
> FieldSetterContext.markPersistentFinalsSet() is not called for
> InputStreamHook based subclasses when user's readObject() calls
> defaultReadObject(), which further means that
> FieldSetter.checkAllFinalsSet() that is eventually called as last thing
> in user's readObject() method might find any persistent finals as not
> set and throw InvalidObjectException.
>
> I have been thinking about this and I see several solutions:
>
> 1. provide protected final methods
> ObjectInputStream.checkPersistentFinalsNotSet() and
> markPersistentFinalsSet() that just delegate to FieldSetterContext for
> ObjectInputStream classes to call as part of their own overriden
> defaultReadObject() method. We should make sure those methods are called
> in JDK's corba InputStreamHook and document they should be called in 3rd
> party subclasses.

Yes, I think I agree with this, but since the FieldSetterContext is 
constructed in OIS private readSerialData, there is nothing to delegate 
to, unless InputStreamHook/IIOPInputStream creates the context.

> 2. in ObjectInputStream constructor, detect if defaultReadObject() is
> overridden and set a flag that makes FieldSetter.checkAllFinalsSet()
> effectively a no-op.

Hmm... maybe, but we I think this could fall foul of JCK. A compliant 
implementation should/must throw if all finals are not set, right?

> 3. 1 + 2 + enable for ObjectInputStream subclasses to declare that they
> obey the contract and that they call markPersistentFinalsSet() from
> overridden defaultReadObject() so checkAllFinalsSet() can be enabled.

Yes, again the problem is that they need to be able to create a field 
setter context.

> What do you think?

I am still thinking about this. I do like your suggestions, but I don't 
see how they will work ( regard to the construction of the context ).

This problem is not unique to OIS, it is a general problem with evolving 
any concrete subclassable class, but I think we do need a reasonable 
solution.

Another possibility is that a default implementation of fieldSetter(), 
in a subclass, could throw UOE, Yuck!

Even if we drop the restriction on checking final field set, or not set, 
there is still the issue of when/how the field context can be created in 
a subclass. I think we may need to expose a factory method for this, 
along with your suggestions above.

-Chris.


> Regards, Peter
>