7 Replies Latest reply on Feb 22, 2008 10:43 PM by Ron Sigal

    Concurrency Issue (ClassCastException) in MicroRemoteClientI

    Paul Mark Skittone Newbie

      JBoss 4.2.1.GA (which includes jboss-remoting 2.2.1.GA)
      Using EJB3 with ClassLoader isolation and Call-By-Value

      We have two ears (A, B) running in the same jboss, which each make a call to a third ear (C), deployed in a second jboss, to retrieve data objects. The class definitions for the data objects are contained in a shared jar file that each ear has in its "lib" directory.

      When A and B both make a call to C at the same time, one of the ears will usually receive a ClassCastException since the data objects that are returned are in the other ear's ClassLoader. Additionally, once this problem occurs, it will continue to occur until the jboss instance is restarted.

      We have made a code change in MicroRemoteClientInvoker, which has almost completely eliminated the ClassCastException. In the few cases where it does still occur, it doesn't cause the jboss to get into the state where it keeps on occurring, so it seems that there is still a problem beyond the below code change.

      MicroRemoteClientInvoker's invoke(InvocationRequest invocationReq) method is, in part, the following in jboss-remoting 2.2.1.GA [and seemingly unchanged in newer versions]:

       UnMarshaller unmarshaller = getUnMarshaller();
      
       // ...marshaller code removed...
      
       if (unmarshaller == null)
       {
       // creating a new classloader containing the remoting class loader (for remote classloading)
       // and the current thread's class loader. This allows to load remoting classes as well as
       // user's classes.
       ClassLoader remotingClassLoader =
       new RemotingClassLoader(getClassLoader(), Thread.currentThread().getContextClassLoader());
      
       // try by locator (in case unmarshaller class name specified)
       unmarshaller = MarshalFactory.getUnMarshaller(getLocator(), getClassLoader());
       if (unmarshaller == null)
       {
       unmarshaller = MarshalFactory.getUnMarshaller(getDataType(), getSerializationType());
       if (unmarshaller == null)
       {
       // went as far as possible to find a unmarshaller, will have to give up
       throw new InvalidMarshallingResource(
       "Can not find a valid unmarshaller for data type: " + getDataType());
       }
       setUnMarshaller(unmarshaller);
       }
       unmarshaller.setClassLoader(remotingClassLoader);
       }
      


      I see two potential problems:
      1) The member variable for the unmarshaller gets set by "setUnMarshaller(unmarshaller);". However, it seems that multiple ears can be using the same instance of the MicroRemoteClientInvoker, so, either there should be no member variable for the unmarshaller (i.e. it should be kept local to the invoke method) or multiple ears shouldn't be using the same MicroRemoteClientInvoker.
      2) MarshalFactory doesn't seem to have any synchronization. I didn't spend much time trying to see if there is a specific problem with the lack of synchronization, but I did synchronize around the entire unmarshaller conditional on the MarshalFactory.class.

      The timing that seems to have been causing the problem is:
      <Call1 and Call2 are each from different ears/ClassLoaders in the same jboss instance. They are both calls to a service in a different jboss.>
      t=0; Call1 gets the local unmarshaller which is null
      t=1; Call1 goes into the code above, gets the unmarshaller, and sets the member variable with "setUnMarshaller(unmarshaller)"
      t=2; Call2 gets the local unmarshaller which is now the same as what Call1 got
      t=3; Call2 does not go into the above conditional since the unmarshaller is not null, so it doesn't call "unmarshaller.setClassLoader(remotingClassLoader);" (*!which means it is using the same classloader as Call1!*)
      t=4; a ClassCastException follows

      The modified code which is working for us is below. The entire conditional synchronizes on MarshalFactory.class (which may not be neccessary) and the "setUnMarshaller(unmarshaller)" is commented out (which keeps the unmarshaller member variable null, so all calls to invoke get the proper unmarshaller and set it with the proper ClassLoader).

       synchronized (MarshalFactory.class) {
       if (unmarshaller == null)
       {
       // creating a new classloader containing the remoting class loader (for remote classloading)
       // and the current thread's class loader. This allows to load remoting classes as well as
       // user's classes.
       ClassLoader remotingClassLoader =
       new RemotingClassLoader(getClassLoader(), Thread.currentThread().getContextClassLoader());
      
       // try by locator (in case unmarshaller class name specified)
       unmarshaller = MarshalFactory.getUnMarshaller(getLocator(), getClassLoader());
       if (unmarshaller == null)
       {
       unmarshaller = MarshalFactory.getUnMarshaller(getDataType(), getSerializationType());
       if (unmarshaller == null)
       {
       // went as far as possible to find a unmarshaller, will have to give up
       throw new InvalidMarshallingResource(
       "Can not find a valid unmarshaller for data type: " + getDataType());
       }
       //setUnMarshaller(unmarshaller);
       }
       unmarshaller.setClassLoader(remotingClassLoader);
       }
       }
      


      I apologize for not having a test case ready, but we're still trying to get everything fully tested and stable in production now that we have the above code changes. Please let us know if any additional information is required. Any help is appreciated.