JBAS-2267 - PooledInvokers missing Version Information

[clebert.suconic]

I'm looking at this issue, and it's not possible to fix Optimized*Stream without break binary compatibility.

I could try adding some exception handling, but it doesn't seem possible.

regular ObjectOutputStream will write a bunch of information with ObjectStreamClass, about what fields are available, and OptimizedOutputStream doesn't. (I guess that's what was the point of optimizing it).

It seems to me this issue should be considered won't fix, as a fix would break binary compatibility, and probably remove the optimization point.

[clebert.suconic]

BTW: I know this forum is about UnifiedInvokers, but i didn't find a better place to discuss about the actual PooledInvoker. Design of EJB Container didn't seem the right place to me.

[starksm64]

The point of JBAS-2267 is that the pooled invoker code is already broken in terms of binary compatibility because we cannot rely on the java serialization rules for binary compatible evolution. A change to the ServerAddress class which should have been compatible was not.

So, leaving the existing streams as is doesn't work. Switching to use the standard object io streams I assume won't work with older versions? Making the streams configurable could allow newer version to revert back to the older behavior, but this also would require custom writeReplace of the ServerAddress to use a legacy version. I'm looking for an option better than won't fix.

[clebert.suconic]

We could add an option on OptimizedObject*Stream to write the complete information.

something like:

protected void writeClassDescriptor(ObjectStreamClass classdesc)
 throws IOException
 {
 if (isCompatibleStream)
 {
 super.writeClassDescriptor(classdesc);
 }
 else
 {
 writeUTF(classdesc.getName());
 }
 }

But my point was, removing that optimization, OptimizedObject*Stream would loose its optimization, and Optimized wouldn't be optimized any more.

I mean, if we keep the compatible streaming here, we will just kill Optimized*Stream

As for ServerClass I tried another approach using GetFields on ServerAddress, and the thing fails before, because the protocol won't know when to stop reading it because ObjectInputStream thinks the ObjectStreamClass matches the data.

[clebert.suconic]

Another option would be make ServerAddress externalizable.
Externalizable classes don't carry information about fields, so it will be our job to make it compatible.

If we keep it broken (if we don't find a better way), at least we will be able to control newer versions by catching IOExdception on writeExternal.

Unfortunately we won't be able to use GetFields on that case either, as Getfiels will be dependent on ObjectStreamClass fields.

[starksm64]

See if you can come up with an Externalizable implementation that works with the older versions.

[clebert.suconic]

I''m trying to create a private readObject using low level protocol operations.

I guess it will work, but I''m still trying.

[clebert.suconic]

It's not possible to read InputStream directly because ObjectInputStream disables its direct buffer access. (What they call BlockedInputStream).

However, I could "hack" this by calling some private methods (using reflection).

I have attached ServerAddress.java into http://jira.jboss.org/jira/browse/JBAS-2267:

Direct url is:
http://jira.jboss.org/jira/secure/attachment/12312030/ServerAddress.java

I would like to hear from Scott about this before taking any action. This was the only possible way (that I found) to fix this.

Basically I have added some static references into reflection:

 static Field binField;
 static Method setBlockModeMethod;

 static
 {
 try
 {
 binField = ObjectInputStream.class.getDeclaredField("bin");
 binField.setAccessible(true);
 Class[] classes = ObjectInputStream.class.getDeclaredClasses();
 for (int i=0;i<classes.length;i++)
 {
 if (classes.getName().equals("java.io.ObjectInputStream$BlockDataInputStream"))
 {
 setBlockModeMethod = classes.getDeclaredMethod("setBlockDataMode",new Class[]{Boolean.TYPE});
 setBlockModeMethod.setAccessible(true);
 break;
 }
 }
 }
 catch (Exception e)
 {
 binField = null;
 setBlockModeMethod = null;
 //e.printStackTrace();
 }
 }

And I'm using these fields to change the block mode.
With the block mode turned of, I can have access to the streaming bytes, and read the fields in the order that I want.

[clebert.suconic]

The proposed solution worked for 4.0.2 as the server, and 4.0.x as the client.

When I places 4.0.x as the server, and 4.0.2 the solution didn't work.

I would have to know on the server, that the client is a 4.0.2, and do something similar.

I will try that approach and update the class. But the solution is going to be pretty similar (with the exception I will be hacking the writeObject also now)

[clebert.suconic]

This is being pretty messy.

If I add a private writeObject on ServerClass, the class is not compatible what means I can't change the streaming on the oposite direction. (newer version->older version).

[clebert.suconic]

I was so blind trying to figure out a way to make Serialization work on this Optimized Stream that I couldn't think in an easiest solution (besides this Rocket Science I was creating). I kind of felt like an idiot, but I guess everybody have these moments :-)

I'm creating SeverAddressExt, and moving clientSocketFactory to its extension.

I'm also creating a System property, that will determine if Server should create a compatible version or the current version.

If compatible version is selected, ServerAddress (without socketFactory) is created.

If not compatible, current version (ServerAddressExt) is then used.

I'm also changing all the Proxies and PooledInvoker to test if the extension is being used. I've added this test to compatibility matrix (locally only for now) and it's passing.


Only thing I need is a better name to the property.

Right now, the user will have to define "pooled-compatible-protocol" as a System property if it's needed compatibility on PooledInvokers between versions.

Another thing I'm doing is writing the classDef regularly if pooled_compatible-protocol is not defined, what will keep version compatible from now on.

[starksm64]

Most our properties follow the dot decimal style of classes so org.jboss.invocation.pooled.protocolVersion or some such is more consistent.

[clebert.suconic]

I'll use org.jboss.invocation.pooled.Legacy

Don't know where to document that yet thought. I will look where to do that later.

[clebert.suconic]

Can I use org.jboss.util.id.SerialVersion to hold the parameter?

[clebert.suconic]

Just talked to Adrian, I will create another class called CompatibilityVersion under org.jboss.invocation.pooled to hold the property