Discussion of JBAS-2436

[clebert.suconic]

I have placed my local changes into http://jira.jboss.com/jira/browse/JBAS-2436

So, I would like someone to evaluate these changes, as a code inspection.

A basic explanation on this:
I have for this IMarshalledValue being added into Commons.
(so, this will require also having RemotingMarshalledValue implementing IMarshalledValue)

There is another pending change on this:

I will add a SerializationManager under JBossAS/invocation that will use MarshalledValueInputStream. It is a simple chnage and I will do after these changes are evaluated.

[adrian.brock]

How do these changes differ from what is on the branch where you were experimenting?

I'm most interested in how interoperability is maintained between 4.0.3 and 4.0.4 :-)
e.g. 4.0.3 client with 4.0.4 and vice versa
e.g. 4.0.3 server with 4.0.4 server sharing marshalled values via JBoss Cache.
How much testing have you done in this area?

[clebert.suconic]

How do these changes differ from what is on the branch where you were experimenting?

When I started merging the changes back, I was trying to diminish the number of changes. So, the branch was an experiment, and these changes are what I'm willing to commit.

I'm most interested in how interoperability is maintained between 4.0.3 and 4.0.4 :-)

I could have UnifiedInvoker from 4.0.3 talking to UnifiedInvoker 4.0.4 (with serializationManager=java) without any problems.

JBoss Cache is instantiating org.jboss.invocation.MarshalledValue directly, so this is not making any usage on the SerializationManager.

The only code I've changed on MarshalledValue was to implement the new interface, so I don't think this would break serialization compatibility. (writeExternal and readExternal are intact for that object)


I will check on the version check testsuite what tests I could add (just to make sure)

[clebert.suconic]

Since Remoting is already released I'm about to commit this issue now.

However I would like to have a little discussion in how users will have to configure JBoss to use this feature.

There are two scenarios configuration:

I - Within UnifiedInvoker, There are no issues here, I think this is pretty easy.

II - On call by value operations
The way I'm doing now, I'm asking for the default serialization on SerializationStreamFactory. So, the user will have to register SerializationStreamFactory as a MBean as the following:

 <mbean code="org.jboss.remoting.serialization.SerializationStreamFactory" name="jboss:service=SerializationManager">
 <attribute name="Manager">org.jboss.remoting.serialization.impl.jboss.JBossSerializationManager</attribute>
 </mbean>

I would like to give an easier configuration way.

I don't know if this is better or not, but what I thought as an option was:

 <list-entity>
 <interceptor>org.jboss.proxy.ejb.ListEntityInterceptor</interceptor>
 <interceptor>org.jboss.proxy.SecurityInterceptor</interceptor>
 <interceptor>org.jboss.proxy.TransactionInterceptor</interceptor>
 <interceptor call-by-value="false">org.jboss.invocation.InvokerInterceptor</interceptor>
 <interceptor call-by-value="true" serialization-provider="jboss">org.jboss.invocation.MarshallingInvokerInterceptor</interceptor>
 </list-entity>

So, if you want to use JbossSerialization in a call-by-value, the user would just provide the serialization provider. I would put java as default with a big comment in how to change it.


So, what about this idea?


Also, where this file is being parsed (standardjboss.xml) in our code base?

[starksm64]

Where are example integration tests for this? We need some solid integration tests if this is going to be used in 4.0.4.

A new issue (http://jira.jboss.com/jira/browse/JBAS-2817) points to the org.jboss.util.stream.IMarshalledValue being in the wrong module and I don't see uses of this in the legacy invocation layer. How is this being used again?

Another related issue since 2817 referes to the pooled invoker streams, is that the pooled invoker streams break java serialization because they are throwing away metadata (http://jira.jboss.com/jira/browse/JBAS-2267).

[clebert.suconic]

We have the regular version matrix testsuite compatible passing (even between 4.0.x and 3.2.8, what means the serialization contract is working properly.

I agree I need to provide a testsuite for JBossSerialization into 4.0.x and marshalledinvocation. I will be working on this as part of this same JIRA.

I will fix 2817 (just adding the interface into applet.jar).

This is being used by Marshalledinvocation. The creation of the MarshalledValue is delegated to SerializationStreamFactory, and I needed the common interface for being able to plug the desirable MarshalledValue acoording to the PluggableSerialization.

The best place to add this was commons. Why do you think this is in the wrong place?

Also, most of the changes I made were made into Unifiedinvoker and JBossRemoting. I don't think JBAS-2267 is related to this issue.

[starksm64]

JBAS-2267 is related because there was a reference to the broken pooled invoker object io streams and I don't want those propagated.

The commons package does not seem to be the correct place for the org.jboss.util.stream.IMarshalledValue interface. Nothing else in that package has any relation to this tagging interface.

The org.jboss.invocation.MarshalledInvocation now has imports from two seperate modules:

import org.jboss.remoting.serialization.SerializationStreamFactory;
import org.jboss.util.stream.IMarshalledValue;

Since it depends on remoting now, why can't the IMarshalledValue in a remoting jar?

[clebert.suconic]

Well... I need to import that interface into Remoting and JBAS.

I needed an integration JAR, and commons was the only place I knew I could do it.


When we have that integration jar, I can move the interface.

[clebert.suconic]

Since it depends on remoting now, why can't the IMarshalledValue in a remoting jar?

That was the initial place...
But that added dependencies on Tomcat, since Tomcat uses MarshalledValue to an use case totally different than invocation.

Adrian "complained" about it, than I moved the interface (and avoided Tomcat depending on Remoting).

[starksm64]

I don't see how tomcat does not depend on remoting anyway due to the import org.jboss.remoting.serialization.SerializationStreamFactory and use of SerializationStreamFactory. The only way it does not is due to lazy loading of class references in code that does not happen to be executed by tomcat. This is a jdk specific behavior.

So this seems to be a tangled mess. It looks like a subclass of MarshalledInvocation that moves the remoting dependencies to the subclass is needed. The MarshalledInvocation class may need to be refactored to provide hooks for the subclass.

[clebert.suconic]

Tomcat does not use MarshalledInvocation.
Tomcat uses MarshalledValue.

This is the import list for MarshalledValue:

import org.jboss.util.stream.IMarshalledValue;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;

import java.util.Arrays;

So, there is no dependency here.


Tomcat needed a MarshalledValue to something else, than they found org.jboss.invocation.MashalledValue and started using it.

If that's a generic class, I guess it shouldn't be under org.jboss.invocation on the first place, or Tomcat should use a different MarshalledValue.


I didn't want to fix Tomcat's dependencies on org.jboss.invocation, but I didn't want to add another dependency on Tomcat.

[starksm64]

Then a subclass of MarshalledValue is needed. No matter how you look at it, IMarshalledValue is a dependency on the new remoting layer.

[starksm64]

Or probably more correctly, IMarshalledValue is a dependency on the new serialization layer.

[clebert.suconic]

The only reason I changed MarshalledValue, was because I wanted a pluggable serialization that would be backard compatible.

There is no dependency between IMarshalledValue and MarshalledInvocation, as there wasn't any dependency between MarshalledValue and MarshalledInvocation before.

Also, If I didn't have a common interface on MarshalledValue, I would have to test for the type everywhere MarshalledValue is used on MarshalledInvocation.

I would have this code instead:

 if (value instanceof MarshalledValue)
 {
 try
 {
 MarshalledValue mv = (MarshalledValue) value;
 value = mv.get();
 }
 catch (Exception e)
 {
 IllegalStateException ise = new IllegalStateException("getValue failed");
 ise.initCause(e);
 throw ise;
 }
 }
 else if (value instanceof IMarshalledValue)
 {
 .... some code here.
 }
 return value;

I actually had this code for a while. IMarshalledValue enabled me to clean up the code a lot.

On places where MarshalledValue is being used outside of the invocation context, still ok to use the same interface as it was used before.

 MarshalledValue value = new MarshalledValue(someObject);

This would be okay as everybody has a dependency on jboss-commons anyway.

Well... this is the best I could think of a way to add pluggable serialization without breaking compatiblity with previous versions and other projects using MarshalledValue outside of the invocation use case. (i.e. HttpSessionStora on Tomcat, and replication on JBossCache).

[clebert.suconic]

IMarshalledValue is a dependency on the new serialization layer.

The way I think is the serialization layer uses a common tag interface to create MarshalledValues, but on Legacy you don't need to change anything.