On 03/25/2015 04:49 PM, Chris Hegarty wrote:
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.

Hi Chris,

Here's how I imagined it. Idea stolen and adapted from parallel-capable ClassLoaders ;-) ...

http://cr.openjdk.java.net/~plevart/jdk9-sandbox/serial-exp-branch/webrev.corba.02/
http://cr.openjdk.java.net/~plevart/jdk9-sandbox/serial-exp-branch/webrev.jdk.02/


This is a variant 3 from above. For ObjectInputStream subclasses that don't override defaultReadObject(), everything should behave as before this change. Those that do override defaultReadObject() have a choice:

- they are *not* finals-tracking capable and checkAllFinalsSet() method only checks that non-persistent finals have been set - they opt-int to be finals-tracking capable by registering and declaring that their overridden defaultReadObject() method either delegates to a super method which *MUST* be finals-tracking capable or explicitly call exposed protected methods: checkPersistentFinalsNotSet()/markPersistentFinalsSet().


That's it.

Haven't tested this yet. Just an idea.

Regards, Peter



Regards, Peter


Reply via email to