10 Replies Latest reply on Dec 1, 2008 8:12 AM by Ales Justin

    AbstractDeploymentClassLoaderPolicyModule is missing reset

    Ales Justin Master

      Deployers's AbstractDeploymentClassLoaderPolicyModule is not removing alias if one was added at AbstractDeploymentClassLoaderPolicyModule construction.

      I've added the following code:

       public abstract class AbstractDeploymentClassLoaderPolicyModule extends ClassLoaderPolicyModule
      @@ -64,12 +66,14 @@
       * Determine the classloading metadata for the deployment unit
       *
       * @param unit the deployment unit
      + * @param addAlias should we add alias or remove
       * @return the classloading metadata
       */
      - private static String determineContextName(DeploymentUnit unit)
      + private static String determineContextName(DeploymentUnit unit, boolean addAlias)
       {
       if (unit == null)
       throw new IllegalArgumentException("Null unit");
      +
       ControllerContext context = unit.getTopLevel().getAttachment(ControllerContext.class);
       if (context == null)
       throw new IllegalStateException("Deployment has no controller context");
      @@ -83,13 +87,21 @@
       Set<Object> aliases = context.getAliases();
       if (aliases != null && aliases.contains(contextName) == false)
       {
      - try
      + Controller controller = context.getController();
      + if (addAlias)
       {
      - context.getController().addAlias(contextName, context.getName());
      + try
      + {
      + controller.addAlias(contextName, context.getName());
      + }
      + catch (Throwable t)
      + {
      + throw new RuntimeException("Error adding deployment alias " + contextName + " to " + context, t);
      + }
       }
      - catch (Throwable t)
      + else
       {
      - throw new RuntimeException("Error adding deployment alias " + contextName + " to " + context, t);
      + controller.removeAlias(contextName);
       }
       }
       }
      @@ -105,7 +117,7 @@
       */
       public AbstractDeploymentClassLoaderPolicyModule(DeploymentUnit unit)
       {
      - super(determineClassLoadingMetaData(unit), determineContextName(unit));
      + super(determineClassLoadingMetaData(unit), determineContextName(unit, true));
       this.unit = unit;
       ControllerContext context = unit.getTopLevel().getAttachment(ControllerContext.class);
       setControllerContext(context);
      @@ -126,4 +138,11 @@
       {
       return CLASSLOADER_STATE;
       }
      +
      + @Override
      + public void reset()
      + {
      + super.reset();
      + determineContextName(unit, false);
      + }
       }
      


      Should this be part of new release for JBoss5 GA?
      I still need to write some tests, checking if it was successfully removed from Controller (in case it was added).

        • 1. Re: AbstractDeploymentClassLoaderPolicyModule is mossing res
          Ales Justin Master

          This

           if (aliases != null && aliases.contains(contextName) == false)
          

          was also a bug.
          It should be
           if (aliases == null || (aliases != null && aliases.contains(contextName) == false))
          


          • 3. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
            Ales Justin Master

            Now I'm seeing this:

            00:38:37,546 ERROR [VFSClassLoaderDescribeDeployer] Error during undeploy: vfszip:/C:/projects/jboss5/trunk/testsuite/output/lib/jboss-seam-numberguess.ear/jboss-seam-numberguess.war
            java.lang.IllegalStateException: Not installed: vfszip:/C:/projects/jboss5/trunk/testsuite/output/lib/jboss-seam-numberguess.ear/jboss-seam-numberguess.war_Alias_AbstractKernelController[9568605]
             at org.jboss.dependency.plugins.AbstractController.uninstall(AbstractController.java:689)
             at org.jboss.dependency.plugins.AbstractController.uninstall(AbstractController.java:568)
             at org.jboss.dependency.plugins.AbstractController.removeAlias(AbstractController.java:621)
             at org.jboss.deployers.plugins.classloading.AbstractDeploymentClassLoaderPolicyModule.determineContextName(AbstractDeploymentClassLoaderPolicyModule.java:104)
             at org.jboss.deployers.plugins.classloading.AbstractDeploymentClassLoaderPolicyModule.reset(AbstractDeploymentClassLoaderPolicyModule.java:146)
             at org.jboss.classloading.spi.dependency.Module.release(Module.java:751)
             at org.jboss.classloading.spi.dependency.ClassLoading.removeModule(ClassLoading.java:71)
             at org.jboss.deployers.plugins.classloading.AbstractClassLoaderDescribeDeployer.undeploy(AbstractClassLoaderDescribeDeployer.java:120)
             at org.jboss.deployers.plugins.classloading.AbstractClassLoaderDescribeDeployer.undeploy(AbstractClassLoaderDescribeDeployer.java:39)
             at org.jboss.deployers.spi.deployer.helpers.AbstractOptionalRealDeployer.internalUndeploy(AbstractOptionalRealDeployer.java:91)
             at org.jboss.deployers.spi.deployer.helpers.AbstractRealDeployer.undeploy(AbstractRealDeployer.java:112)
             at org.jboss.deployers.plugins.deployers.DeployerWrapper.undeploy(DeployerWrapper.java:196)
             at org.jboss.deployers.plugins.deployers.DeployersImpl.doUndeploy(DeployersImpl.java:1469)
             at org.jboss.deployers.plugins.deployers.DeployersImpl.doUninstallParentLast(DeployersImpl.java:1376)
             at org.jboss.deployers.plugins.deployers.DeployersImpl.doUninstallParentLast(DeployersImpl.java:1356)
             at org.jboss.deployers.plugins.deployers.DeployersImpl.uninstall(DeployersImpl.java:1331)
             at org.jboss.dependency.plugins.AbstractControllerContext.uninstall(AbstractControllerContext.java:354)
             at org.jboss.dependency.plugins.AbstractController.uninstall(AbstractController.java:1631)
             at org.jboss.dependency.plugins.AbstractController.uninstallContext(AbstractController.java:1242)
             at org.jboss.dependency.plugins.AbstractController.change(AbstractController.java:827)
             at org.jboss.dependency.plugins.AbstractController.change(AbstractController.java:553)
             at org.jboss.deployers.plugins.deployers.DeployersImpl.process(DeployersImpl.java:694)
             at org.jboss.deployers.plugins.main.MainDeployerImpl.process(MainDeployerImpl.java:545)
             at org.jboss.profileservice.management.upload.remoting.DeployHandler.stop(DeployHandler.java:251)
             at org.jboss.profileservice.management.upload.remoting.DeployHandler.invoke(DeployHandler.java:153)
             at org.jboss.remoting.ServerInvoker.invoke(ServerInvoker.java:908)
             at org.jboss.remoting.transport.socket.ServerThread.completeInvocation(ServerThread.java:742)
             at org.jboss.remoting.transport.socket.ServerThread.processInvocation(ServerThread.java:695)
             at org.jboss.remoting.transport.socket.ServerThread.dorun(ServerThread.java:522)
             at org.jboss.remoting.transport.socket.ServerThread.run(ServerThread.java:230)

            :-(

            • 5. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
              Ales Justin Master

              Found it. ;-)

              The problem is, that we're releasing Module twice:
              - AbstractClassLoaderDescribeDeployer::undeploy
              - AbstractLevelClassLoaderSystemDeployer::undeploy

              Which one is overhead?
              Or should we have two phases of remove?

              • 6. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
                Ales Justin Master

                 

                "alesj" wrote:

                - AbstractLevelClassLoaderSystemDeployer::undeploy (ALCLSD)

                Removing Module::reset from ALCLSD breaks this test:
                - MockClassLoaderDependenciesUnitTestCase::testADependsUponModuleBRedeployB


                • 7. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
                  Ales Justin Master

                  What about if I simply override Module::release (instead of reset) in AbstractDeploymentClassLoaderPolicyModule:

                   @Override
                   public void release()
                   {
                   try
                   {
                   super.release();
                   }
                   finally
                   {
                   determineContextName(unit, false);
                   }
                   }
                  

                  All the tests pass, and I don't get any error log. :-)

                  • 8. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
                    Adrian Brock Master

                     

                    "alesj" wrote:
                    Found it. ;-)

                    The problem is, that we're releasing Module twice:
                    - AbstractClassLoaderDescribeDeployer::undeploy
                    - AbstractLevelClassLoaderSystemDeployer::undeploy

                    Which one is overhead?
                    Or should we have two phases of remove?


                    I don't think that is true.
                    The first one does the release() the second one only does reset()
                    which removes the cached state that will get recalculated on redeploy.

                    The alias removal should be done at release.

                    • 9. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
                      Ales Justin Master

                       

                      "adrian@jboss.org" wrote:

                      I don't think that is true.
                      The first one does the release() the second one only does reset()

                      You're partly right.
                      It's true that the 1st one does release,
                      but it's then the release that also invokes reset.
                       /**
                       * Release the module
                       */
                       public void release()
                       {
                       Domain domain = this.domain;
                       if (domain != null)
                       domain.removeModule(this);
                       reset();
                       }
                      

                      ;-)

                      • 10. Re: AbstractDeploymentClassLoaderPolicyModule is missing res
                        Ales Justin Master

                        So, we're fine with my fix - overriding release?

                        And release calling reset?
                        I guess that's OK, as we shouldn't rely that reset is called before?

                        I'll do another release (pun? :-),
                        2.0.3 this time, as I scre... up 2.0.2. :-(