Roshan,
Comments inline below.
-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?
Your recommended changes are an improvement for replacing Class.forName.
I would like to add that the Class.forName code was only added for
compatibility for running against JDK 5.
In JDK 6 and higher, the code is using ServiceLoader.load not the
Class.forName.
Shoal is being loaded via OSGi in Glassfish 3.1 and higher where the
minimal JDK requirement is JDK 6.
> 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?
It is preferable to remove the single use of sun.security.action.
Simply calling System.getProperty() will be fine.
I do not understand what making sun.security.action optional would do
when it was not available to load. From the code point
of view, the line that uses it, it is not optional that the class be
loaded. There is no handling for when it is not available.
So I would be more comfortable with removing the usage of
sun.security.action and remove optional import reference from OSGi Manifest.
> 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
>>
>>