I created a test for DynamicImport-Package to check whether https://jira.jboss.org/jira/browse/JBCL-131 is actually working at the OSGi layer.

I fail to see how dynamic import and JBCL-131 are related?

JBCL-131 is an impl of OSGi' lazy bundle activation -- e.g. only invoke BundleActivator if some class from that bundle needs to be loaded.

Ales Justin wrote:

 

I created a test for DynamicImport-Package to check whether https://jira.jboss.org/jira/browse/JBCL-131 is actually working at the OSGi layer.

I fail to see how dynamic import and JBCL-131 are related?

JBCL-131 is an impl of OSGi' lazy bundle activation -- e.g. only invoke BundleActivator if some class from that bundle needs to be loaded.

Well actually JBCL-131 is about lazy resolve, not lazy start.

It's not working because it hasn't been enabled.

I've committed the following patch which makes it work using Ales's DeploymentMetaData

(I've not done anything with lazy start).

I don't know if that is the correct place for it?

Another question is whether you always want to enable lazy resolve for OSGi, even if the user specifies something else?

P.S. There's no point re-opening issues in closed releases. You need to raise new issues.

/core/src/main/java/org/jboss/osgi/framework/deployers
$ svn diff
Index: AbstractClassLoadingDeployer.java
===================================================================
--- AbstractClassLoadingDeployer.java    (revision 103238)
+++ AbstractClassLoadingDeployer.java    (working copy)
@@ -29,6 +29,7 @@
import org.jboss.classloading.spi.metadata.CapabilitiesMetaData;
import org.jboss.classloading.spi.metadata.ClassLoadingMetaData;
import org.jboss.classloading.spi.metadata.RequirementsMetaData;
+import org.jboss.deployers.plugins.classloading.DeploymentMetaData;
import org.jboss.deployers.spi.DeploymentException;
import org.jboss.deployers.spi.deployer.DeploymentStages;
import org.jboss.deployers.spi.deployer.helpers.AbstractSimpleRealDeployer;
@@ -61,7 +62,10 @@
    {
       super(OSGiMetaData.class);
       addInput(AbstractBundleState.class);
-      setOutput(ClassLoadingMetaData.class);
+      addInput(ClassLoadingMetaData.class);
+      addInput(DeploymentMetaData.class);
+      addOutput(ClassLoadingMetaData.class);
+      addOutput(DeploymentMetaData.class);
       setStage(DeploymentStages.POST_PARSE);
       setTopLevelOnly(true);
    }
@@ -79,6 +83,13 @@
    @Override
    public void deploy(DeploymentUnit unit, OSGiMetaData osgiMetaData) throws DeploymentException
    {
+      if (unit.isAttachmentPresent(DeploymentMetaData.class) == false)
+      {
+         DeploymentMetaData deploymentMetaData = new DeploymentMetaData();
+         deploymentMetaData.setLazyResolve(true);
+         unit.addAttachment(DeploymentMetaData.class, deploymentMetaData);
+      }
+     
       if (unit.isAttachmentPresent(ClassLoadingMetaData.class))
          return;

Adrian Brock wrote:

I've committed the following patch which makes it work using Ales's DeploymentMetaData
<snip/>
+import org.jboss.deployers.plugins.classloading.DeploymentMetaData;

P.S.2 Shouldn't this metadata be in the spi?

Adrian Brock wrote:

 

Adrian Brock wrote:

I've committed the following patch which makes it work using Ales's DeploymentMetaData
<snip/>
+import org.jboss.deployers.plugins.classloading.DeploymentMetaData;

P.S.2 Shouldn't this metadata be in the spi?

I guess for this one it's not too late to be moved there -- as it's been only introduced in 2.2.0.AlphaX?

(dunno why I put it there in the first place, might be same stupid reason as DepenenciesMD)

What about DependenciesMetaData -- which is also in impl/plugins?

Since this one has been around longer. :-(

I guess for this one it's not too late to be moved there -- as it's been only introduced in 2.2.0.AlphaX?

https://jira.jboss.org/jira/browse/JBDEPLOY-255

Thomas Diesler wrote:

 

This change causes regression in bundle lifecycle handling. I added test coverage for this in the framework's BundleLifecycleTestCase

<snip/>

I still need to look into the details of why this would happen.

It's happening because B's deployment unit has required-stage of "CLASSLOADER" (RESOLVED in OSGi) after the start() failed.

i.e. it wants to resolve it.

So when you install bundleX, it also says, hey B is not in the correct state and I can resolve it now,

but only by moving X to ClassLoader as well.

That is what lazy resolve is all about. It automatically resolves other classloaders that it needs if they are not resolved.

If B went back to required-stage "DESCRIBE" (INSTALLED in OSGi) after the start() failed then your test would work as expected.

I worked this out by adding the following to the test after the b.start() failure:

         OSGiBundleState bs = (OSGiBundleState) getBundleManager().getBundleState(bundleB);
         System.out.println(bundleB.getSymbolicName() + " state=" + ConstantsHelper.bundleState(bundleB.getState()) + " required=" + bs.getDeploymentUnit().getRequiredStage());

Which produces:

lifecycle-bundleB state=INSTALLED required=ClassLoader

So you can see the bundle state is inconsistent with the deployment unit state.

This patch resolves the issue. It was only resetting the state if DeployerClient.change() threw a DeploymentException,

while in this case, there's no exception it is just incomplete.

core/src/main/java/org/jboss/osgi/framework/packageadmin
$ svn diff
Index: PackageAdminImpl.java
===================================================================
--- PackageAdminImpl.java    (revision 103305)
+++ PackageAdminImpl.java    (working copy)
@@ -347,6 +347,18 @@
       {
          log.error("Error resolving bundles: " + resolvableBundles, ex);
          allResolved = false;
+        
+         // Reset the required state for bundles that didn't get resolved
+         for (OSGiBundleState bundleState : resolvableBundles)
+         {
+            if (bundleState.getState() == Bundle.INSTALLED)
+            {
+               DeploymentUnit unit = bundleState.getDeploymentUnit();
+               unit.setRequiredStage(DeploymentStages.DESCRIBE);
+               ControllerContext ctx = unit.getAttachment(ControllerContext.class);
+               ctx.setRequiredState(ControllerState.newState(DeploymentStages.DESCRIBE.getName()));
+            }
+         }
       }
      
       return allResolved;

+               ctx.setRequiredState(ControllerState.newState(DeploymentStages.DESCRIBE.getName()));

This should be ControllerState.getInstance instead of ::newState, althought impl details will show it's the same result at the end. :-)

To pass one of the other tests you added, I had to change the OSGi classloader shutdown policy to GC.

I thought you'd done that a while ago, but apparantly not (at least I couldn't find it?)

Index: AbstractClassLoadingDeployer.java
===================================================================
--- AbstractClassLoadingDeployer.java    (revision 103305)
+++ AbstractClassLoadingDeployer.java    (working copy)
@@ -26,6 +26,7 @@
import java.util.List;

import org.jboss.classloader.spi.ClassLoaderDomain;
+import org.jboss.classloader.spi.ShutdownPolicy;
import org.jboss.classloading.spi.metadata.CapabilitiesMetaData;
import org.jboss.classloading.spi.metadata.ClassLoadingMetaData;
import org.jboss.classloading.spi.metadata.RequirementsMetaData;
@@ -100,6 +101,7 @@
       OSGiBundleManager bundleManager = bundleState.getBundleManager();

       OSGiClassLoadingMetaData classLoadingMetaData = new OSGiClassLoadingMetaData();
       classLoadingMetaData.setName(bundleState.getSymbolicName());
       classLoadingMetaData.setVersion(bundleState.getVersion());
       classLoadingMetaData.setDomain(domain != null ? domain.getName() : null);

+      classLoadingMetaData.setShutdownPolicy(ShutdownPolicy.GARBAGE_COLLECTION);

In case you missed it:

http://community.jboss.org/thread/147089

http://community.jboss.org/thread/148829