Roshan,
Sorry to ignore this review request.
I was busy with Glassfish 3.1.2 release issues.
Starting to look it over now.
-Joe
On 1/4/12 3:23 PM, Roshan A. Punnoose wrote:
> Sorry this took so long but I had to add a few more changes to get it working better in a pure osgi environment. I wanted to run these changes by you to get your ideas.
>
> 1. As we discussed before, I added the change to use a custom NetworkManager
> 2. Had to fix one or two places where Class.forName is being used. This is not a good practice in OSGi bundles since it is using a different ClassLoader. I just changed it to the use the current context class loader. What do you think?
> 3. I made the use of sun.security.action in the OSGi Manifest to be optional since I couldn't get it loaded into Felix otherwise. What do you think?
>
> Overall here is the diff of the changes:
> Index: impl/src/main/java/com/sun/enterprise/mgmt/transport/AbstractNetworkManager.java
> ===================================================================
> --- impl/src/main/java/com/sun/enterprise/mgmt/transport/AbstractNetworkManager.java (revision 1731)
> +++ impl/src/main/java/com/sun/enterprise/mgmt/transport/AbstractNetworkManager.java (working copy)
> @@ -196,7 +196,7 @@
> NetworkManager networkManager = null;
> // for jdk 5. just use class loader.
> try{
> - Class networkManagerClass = Class.forName(classname);
> + Class networkManagerClass = Thread.currentThread().getContextClassLoader().loadClass(classname);
> networkManager = (NetworkManager)networkManagerClass.newInstance();
> } catch (Throwable x) {
> LOG.log(Level.SEVERE, "fatal error instantiating NetworkManager service", x);
> @@ -211,6 +211,10 @@
> } catch (Throwable t) {
> // jdk 5 will end up here.
> }
> + //Allowing for a custom NetworkManager
> + if (transport != null&& transport.contains(".")) {
> + networkManager = findByClassLoader(transport);
> + }
> if (networkManager == null) {
> String classname = null;
> if (transport.compareToIgnoreCase(GMSConstants.GRIZZLY_GROUP_COMMUNICATION_PROVIDER) == 0) {
> Index: impl/pom.xml
> ===================================================================
> --- impl/pom.xml (revision 1731)
> +++ impl/pom.xml (working copy)
> @@ -110,6 +110,7 @@
> <extensions>true</extensions>
> <configuration>
> <instructions>
> +<Import-Package>sun.security.action;resolution:=optional,*</Import-Package>
> <Export-Package>com.sun.enterprise.ee.cms.impl.*;com.sun.enterprise.gms.tools;com.sun.enterprise.ee.cms.logging;com.sun.enterprise.mgmt.*;com.sun.enterprise.osgi</Export-Package>
> </instructions>
> </configuration>
> Index: project.properties
> ===================================================================
> --- project.properties (revision 1731)
> +++ project.properties (working copy)
> @@ -58,7 +58,7 @@
> shoal.jar.name=${shoal.image.name}-${shoal.image.version}
>
> # fix hardcoding this. ideal to get this from the hudson build process.
> -shoal.gms.version=1.5.34
> +shoal.gms.version=1.5.36
>
> # Where dependencies will be stored.
> appserv.repo.hostname=koori.sfbay.sun.com
> Index: api/src/main/java/com/sun/enterprise/ee/cms/core/GMSFactory.java
> ===================================================================
> --- api/src/main/java/com/sun/enterprise/ee/cms/core/GMSFactory.java (revision 1731)
> +++ api/src/main/java/com/sun/enterprise/ee/cms/core/GMSFactory.java (working copy)
> @@ -269,8 +269,8 @@
> private static GroupManagementService findByClassLoader(String classname) {
> GroupManagementService gmsImpl = null;
> // for jdk 5. just use class loader.
> - try {
> - Class GmsImplClass = Class.forName(classname);
> + try {
> + Class GmsImplClass = Thread.currentThread().getContextClassLoader().loadClass(classname);
> gmsImpl = (GroupManagementService) GmsImplClass.newInstance();
> if (gmsImpl == null) {
> LOG.log(Level.SEVERE, "factory.load.service.error");
>
> On Dec 21, 2011, at 10:18 AM, Joseph Fialli wrote:
>
>> Comments inline below.
>>
>> -Joe
>>
>> On 12/15/11 9:25 AM, rpunnoose_at_proteus-technologies.com wrote:
>>> Hi,
>>>
>>> I really have two questions:
>>>
>>> 1. I want to use my own custom transport (networkmanager, not grizzly
>>> or jxta). I think I can just implement the Networkmanager interface and
>>> make it available. But looking at the AbstractNetworkManager code, it
>>> doesn't seem like either the findByServiceLoader or findByClassLoader
>>> (from the getInstance) will work with custom transports. Any ideas?
>>>
>> To allow for custom transports,
>> AbstractNetworkManager.findByServiceLoader and
>> AbstractNetworkManager.getInstance
>> could be extended that if the transport contains ".", treat it as a
>> fully qualified implementation class. Current
>> implementation only allows for transport to be convenient enumeration of
>> grizzly1_9, grizzly2_0 and rest to default
>> to jxta. But adding a check for "." in the transport name could treat
>> the transport as a fully qualified
>> classname and to just look up that value directly using serviceloader in
>> JDK 6 and higher and class loading in JDK 5.
>>
>>> 2. I want to use shoal in osgi, and have the custom transport as a
>>> separate bundle. The ServiceLoader approach will not work with osgi. Is
>>> there another approach I can take? I think I can create an OSGi version
>>> extension (just extend the AbstractNetworkManager) that would look at
>>> OSGi services as well. Any ideas?
>> Shoal is being loaded as an OSGi module in GlassFish since GlassFish 3.1.
>> So you can already use Shoal in osgi. The targets are generated by
>> mvn install. The OSGi jars in shoal workspace are generated in
>> shoal/gms/api/target/shoal-gms-api.jar
>> and shoal/gms/impl/target/shoal-gms-impl.jar and
>> shoal/cache/target/shoal-cache.jar.
>> Here is mvn repository for the OSGi loadable shoal jars:
>> https://maven.java.net/content/repositories/releases/org/shoal/.
>>
>> So while Shoal is not using OSGi to load its dependencies,
>> it can be loaded as an OSGi module.
>>> Thanks much!
>>>
>>> Roshan
>> --
>> Follow this link to mark it as spam:
>> http://mailfilter.proteus-technologies.com/cgi-bin/learn-msg.cgi?id=32E7A28486.A762F
>>
>>