brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From neykov <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: OSGi class loading fixes
Date Mon, 28 Jul 2014 11:56:06 GMT
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/90#discussion_r15458083
  
    --- Diff: core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java ---
    @@ -139,15 +138,16 @@ public InternalEntityFactory(ManagementContextInternal managementContext,
Entity
             }
             builder.addAll(spec.getAdditionalInterfaces());
             Set<Class<?>> interfaces = builder.build();
    -        
    -        // TODO OSGi strangeness! The classloader obtained from the type should be enough.
    -        // If an OSGi class loader, it should delegate to find things like Entity.class
etc.
    -        // However, we get errors such as:
    +
    +        // When using the entity's classloader, we get errors such as:
             //    NoClassDefFoundError: brooklyn.event.AttributeSensor not found by io.brooklyn.brooklyn-test-osgi-entities
             // Building our own aggregating class loader gets around this.
    -        // But we really should not have to do this! What are the consequences?
    +        //
    +        // The reason for the error is that the proxy tries to load all classes
    +        // referenced from the entity and its interfaces with the single passed loader
    +        // while a normal class loading would nest the class loaders (loading interfaces'
    +        // references with their own class loaders which in our case are different).
             AggregateClassLoader aggregateClassLoader =  AggregateClassLoader.newInstanceWithNoLoaders();
    -        aggregateClassLoader.addFirst(classloader);
    --- End diff --
    
    Looking at the code I didn't find a case where spec.getImplementation() != enttiy.getClass()
- the entity is always instantiated from getImplementation() if available. 
    Event if that's the case and the classes (and possibly the classloaders) differ why would
we want to use the classloader of a class which didn't instantiate the entity instance?
    Looking at this code again I think that it should be improved but in another direction
- adding the classloaders of all extended classes and interfaces recursively, not only of
the explicitly specified interfaces. WDYT?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message