JBossDeveloper

Dan Sirbu wrote:

 

Hi Brian,

 

   I have taken a break from the subject and after giving multiple taughts I still think there is a "flaw". Here is my view based on my current understanding:

 

   I can have, let's say, two kind of modules: one that implements the module ifc thus an extension && one that does not. Let's name : ModuleExtension the one that has defined an interface towards the : org.jboss.as.controller.Extension ifc; and let's name Module the one that does not have such a definition.

  What I understand is that the ModuleExtension by design should not have any other service defined - which is OK. And if it does it is discarded - which again is OK.

That's not really true.  Neither JBoss Modules nor JBoss AS will actively browse modules for services as this would be a significant performance drain to little advantage.  So services are never "discarded", they're simply ignored until someone asks for them.

Dan Sirbu wrote:

 

  What I also understand is that the Module by design can have a service defined. Now the service defined into Module is available to any war, rar, etc... file that is deployed and have somehow a dependecy defined towards that Module (see prev dicussions on how one can achieve that). And the war file, for example, uses the 'regular' Service.load instead of using the jboss-module one - which is what one would expect.

  So far so good.

I see a couple items of confusion here.  First any module which provides an AS extension must be in the static module repository, because extensions are initialized during AS boot and there are no deployments at this early stage.  Second, the ServiceActivator interface (which is what I assume you're referring to as having a "service defined") is only read by deployment descriptors, so any ServiceActivators in a static modue will be ignored.  However, an extension module can define services through other APIs so this is not really a limitation.

Dan Sirbu wrote:

 

  Now, let's say that ModuleExtension needs Module for whaterver reason. Then one would go into "module.xml" of ModuleExtension and add a dependency towards that Module. Then, what I would expect as behaviour is that the Module service should be available to ModuleExtension via the 'regular' Service.load and not via the jboss-module service loader !

Okay here's another point of confusion.  The JBoss Modules service loader is the "regular" ServiceLoader, just wrapped up in a helper which provides the module's class loader for you.  This point of confusion seems to be masking the real problem.  If you want your extension module to "see" the service provided by its module dependency, you simply add 'services="import"' to the dependency element for that dependency.  If you want other modules which import the extension module to "see" the services provided by that dependency, then instead use 'services="export"'.  That's it; this will make it work whether you're using Module.loadService() or ServiceLoader.

Dan Sirbu wrote:

 

Hi Brian,

 

Since Module is not defined as a jboss-module extension, then the ModuleExtension should be able to have access to the Module service interface via the 'regular' Service.load and that because, to me, the Module should be in the classpath of the ModuleExtension one such a dependency has been created. There is no real need to go towards jboss-module service loader - at least I do not see one.

As I have said, the two are one and the same.

Dan Sirbu wrote:

 

  If a war file, for example, would need access to ModuleExtension it would add a dependecy towards it. The dependency should ensure that the objects defined with the ModuleExtension become visible to the war file including the ones from the Module meaning that the classpath of the ModuleExtension would be made available to the war. At least that is I think a dependency should do.

This is only done when the service is explicitly declared like above.  If we did not filter these by default, then every module would see tons and tons of extra services which may or may not actually be desired and might actually be completely wrong.  However we do allow services to be shared across dependencies already as I've described, which neatly solves the problem.

Here's my guess at to what is going on. If this doesn't describe what you're doing, then never mind.

You have a class ModuleExtensionClass that's in your ModuleExtension module. And another class ServiceAPIClass that's either in ModuleExtension or Module.

In ModuleExtensionClass.initialize(ExtensionContext) you are calling

ServiceLoader.load(ServiceAPIClass.class)

If that call is failing with a CNFE, it's a sign that the module dependencies aren't set up right. But my theory is that's not what's happening.

If I'm correct so far, does using this call work?

ServiceLoader.load(ServiceAPIClass.class, ModuleExtensionClass.class.getClassLoader())

The single arg variant of ServiceLoader.load is the same as calling ServiceLoader.load(Class c, Thread.currentThread().getContextClassLoader()); In AS 7.0.0/7.0.1 when Extension.initialize is called the TCCL is null, so you're not going to get the correct classloader.

We have changed this in master, so in 7.0.2 when Extension.initialize is called, the TCCL will the defining classloader of the Extension impl (i.e. ModuleExtensionClass.class.getClassLoader()). So, if my theory is right you should find this works in a current build of master but not in 7.0.1.

Brian Stansberry wrote:

 

And another class ServiceAPIClass that's either in ModuleExtension or Module.

Actually, it would have to be in Module or it wouldn't be visible to the class that implements it.

Yes, we completely agree that the java.util.ServiceLoader API is the correct one to use. You shouldn't have to use any jboss-modules API.

The difference between this case and an EAR/RAR/WAR relates to the reliance on the thread context class loader. In an EE application,  what the TCCL should be is clearly mandated in the EE spec. There is a contract the application code can rely on. In the case of an Extension there was no such contract. In the absence of a contract, the TCCL could be anything. In that kind of situation it's always better to use the 2 arg variant of java.util.ServiceLoader.load().

I apologize if this issue has caused your team frustration. During development of AS7 we didn't have cases of anyone using Extension in this way, so how the TCCL was set did not show up as an issue. Now that we see that extension developers expect the TCCL to be set differently than we were doing it, we've changed the AS to set the TCCL to the Extension implementation's classloader. I'll change the Extension javadoc to state that, which will make this an official contract that can be relied upon.

Dan Sirbu wrote:

 

The issue to me is that if the ModuleExtension needs a service from another package that was never intended to be an extention, thus a Module only, then when I want to  access a service from that Module I should NOT need to use the jboss-module ServiceLoader API !

I should be able to use the java.util.ServiceLoader as does any WAR/RAR or other type of app deployed as long as the dependency is set properly!

I just want to make it clear that you do NOT need to use the jboss modules service loader API.  This has never been required, nor will it ever be required.  This API exists as a convenience only.

As Brian said, you should always be using the two-argument form of java.util.ServiceLoader#load() and provide your own class loader with it (I don't know what you're trying to accomplish with the getParent() but it's a safe bet that it's not the right thing to do).  Usually this is just "getClass().getClassLoader()", though you'll have to use a class literal in the static method case.  Relying on the thread context class loader is never a good idea under any circumstances.  If you rely on TCCL by using the one-argument form, you're setting yourself up for failure at some point, either here and now or in the future.  If you use the two-argument form, your program will always work as long as your class loader has the proper dependencies.

To apply some more color, we have introduced a TCCL contract (What Brian was reffering to) in upstream master (which will be released in 7.0.2).

This will make it much easier to deal with as you mention legacy code, and also JDK APIs which just were not designed for a modular world.

As of 7.0.2:

• The TCCL during any MSC Service will be the defining classloader
• The TCCL during any management operation (includes extensions) will be the defining CL

So in a nutshell single arg ServiceLoader will work in 7.0.2. Although maybe you want to try our local master before that?

Dan Sirbu wrote:

 

Hi Jason,

 

   I think I can give it a try - it will be slower since I am not familiar in building the full jboss as7 release and the other steps. But I can give it a try.

 

BR,

Dan S.

FYI we have a latest binary build you can always grab here:

http://hudson.jboss.org/jenkins/view/JBoss%20AS/job/JBoss-AS-7.0.x/lastSuccessfulBuild/artifact/build/target/jboss-7.0.x.zip

We're definitely getting on the same page, and I'm glad we're discussing in detail; it's worthwhile for me, hopefully for you, hopefully for others who may read this.

Jason's already pointed out that 7.0.2 should solve the practical issue. We've made the caller setting the TCCL a part of the contract of the Extension interface.

A lot of the rest of what I've said and what David has said is more oriented toward best practices and such. One thing the AS7 team has encountered a lot in the course of developing AS 7 is frameworks (some of which we helped write sometime in the past) that make assumptions about what the TCCL will be in the absence of a contract. So we're always trying to spread the word about being careful about making that assumption.

Dan Sirbu wrote:

 

  Now - if you ask me to add a second argument to ServiceLoader in Module#1 - then there is a no go - because Module#1 may be a 3rd party product that I cannot change, as example. Thus the ServiceLoader.load with one argument is the only option that can be available.

Sounds like you're in the same boat we are in. You have unknown code that you are calling into that is making assumptions about the TCCL. In this case before calling into it you can set the TCCL to the correct classloader, hopefully one defined in the ServiceAPIClass contract. That could very well be the defining classloader for Module#1's impl of the service interface.

The classloader for the ModuleExtension module, which in 7.0.2 will be the TCCL, may not have visibility to classes in Module#2. That's the nature of modular classloading -- the intent is to provide proper isolation, and not to expose everything to everybody. So if you think Module#1 is going to be relying on the TCCL having visibility to everything Module#1 depends on, then you should set the TCCL to a classloader that should have that visibility. Most likely that is Module#1's classloader.

So the scenario is

ME -> M1 -> M2 -> M3

with "->" meaning <dependencies><module name="X"/>... in the left side module's module.xml and no 'export="true"' attribute. So ME can only see M1's classes, M1 can only see M2, M2 can only see M3.

If M2 uses the single param java.util.ServiceLoader.load() variant and doesn't do anything to change the TCCL, this setup will not work. In 7.0.2 when M2 is executing the TCCL will have visibility to the classes in ME and M1 (if ME doesn't change the TCCL from what the AS passes in) or to the classes in M1 and M2 (if ME changes the TCCL to M1's classloader.) Either way, M3 classes are not visible.

Options:

1) If ME, M1, M2, M3 are really a unit, package ME.jar, M1.jar, M2.jar, M3.jar in a single module.

2) In ME's module.xml's dependencies section, add

<module name="M1"/>

<module name=M2"/>

<module name="M3"/>

The TCCL passed in by AS 7.0.2.+ will be able to see all relevant classes.

3) In M2's module.xml have <module name="M3" export="true"/> and in M1's module.xml have <module name="M2" export="true"/>

Again, the TCCL passed in by the AS will be able to see all relevant classes. If some other module depends on M1, the M2 and M3 classes will be visible to that module as well. Which may be a good thing or a bad thing. If it's bad, solution 2) is better.

4) If M1, M2, M3 are really a unit, package M1.jar, M2.jar, M3.jar in a single module.

5) In M1's module.xml's dependencies section, add

<module name=M2"/>

<module name="M3"/>

Then in ME, change the TCCL to M1's classloader before calling into M1. When M1 calls into M2, the TCCL will be one that can see M3's classes.

6) In M2's module.xml have <module name="M3" export="true"/>. M3's classes will now be visible to M1. Then in ME, change the TCCL to M1's classloader before calling into M1. When M1 calls into M2, the TCCL will be one that can see M3's classes. If some other module depends on M2, the M3 classes will be visible to that module as well. Which may be a good thing or a bad thing.

7) Package M2.jar and M3.jar in a single module, M2+. Then in ME, change the TCCL to M1's classloader before calling into M1. When M1 calls into M2+, the TCCL will be one that can see M3.jar's classes. If some other module depends on M2+, the M3.jar classes will be visible to that module as well. Which may be a good thing or a bad thing.

Dan Sirbu wrote:

 

     In a genric way, once in a Module, the Module should have visibility on all other Modules that were added as dependecies in the ModuleExtension, to cover more complex stuff.

This can be done in a variety of ways, as described above. What can't be done is having a module's dependencies exposed without the consent of the module. A module's dependencies are treated as internal details unless the export="true" attribute is added.