Registering listener on KernelEventManager

[starksm64]

I'm looking at providing a better startup notification as the current one from the ServerImpl occurs just at the end of the bootstrap phase. I'm trying to test out an example listener, but the following

<deployment xmlns="urn:jboss:bean-deployer:2.0">
 <bean name="StartupMonitor"
 class="org.jboss.test.profileservice.lib.startup.StartupMonitor">
 <install bean="jboss.kernel:service=KernelEventManager" method="registerListener">
 <!-- registerListener(Object name, KernelEventListener listener, KernelEventFilter filter, Object handback) -->
 <parameter>jboss.kernel:service=KernelEventManager</parameter>
 <parameter><this/></parameter>
 <parameter><null/></parameter>
 <parameter><null/></parameter>
 </install>
 </bean>
</deployment>

fails with an error like:

15:38:32,904 ERROR [AbstractKernelController] Error installing to Installed: name=StartupMonitor state=Start
java.lang.IllegalArgumentException: Cannot install, context BeanKernelRegistryEntry@56d0a8c{target=org.jboss.kernel.plugins.event.AbstractEventManager@39cf701c} does not implement InvokeDispatchContext
 at org.jboss.kernel.plugins.dependency.InstallAction.installActionInternal(InstallAction.java:79)
 at org.jboss.kernel.plugins.dependency.KernelControllerContextAction.installAction(KernelControllerContextAction.java:135)
 at org.jboss.kernel.plugins.dependency.KernelControllerContextAction.installAction(KernelControllerContextAction.java:46)
 at org.jboss.dependency.plugins.action.SimpleControllerContextAction.simpleInstallAction(SimpleControllerContextAction.java:62)
 at org.jboss.dependency.plugins.action.AccessControllerContextAction.install(AccessControllerContextAction.java:71)
 at org.jboss.dependency.plugins.AbstractControllerContextActions.install(AbstractControllerContextActions.java:51)
 at org.jboss.dependency.plugins.AbstractControllerContext.install(AbstractControllerContext.java:327)
 at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:1309)
 at org.jboss.dependency.plugins.AbstractController.incrementState(AbstractController.java:734)
 at org.jboss.dependency.plugins.AbstractController.resolveContexts(AbstractController.java:862)
 at org.jboss.dependency.plugins.AbstractController.resolveContexts(AbstractController.java:784)
 at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:574)
 at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:398)
 at org.jboss.deployers.vfs.deployer.kernel.BeanMetaDataDeployer.deploy(BeanMetaDataDeployer.java:69)
 at org.jboss.system.deployers.TempBeanMetaDataDeployer.deploy(TempBeanMetaDataDeployer.java:48)
 at org.jboss.system.deployers.TempBeanMetaDataDeployer.deploy(TempBeanMetaDataDeployer.java:35)

Why wouldn't the event mgr have an InvokeDispatchContext implementation?

[alesj]

Why wouldn't the event mgr have an InvokeDispatchContext implementation?

At the beginning AbstractKernelRegistryEntry had no notion of dispatch.
When I was changing KernelBus I added the attribute dispatch notion to it.
Dunno why I didn't add full invoke dispatch.
I can do that now, since the BeanKernelRegistryEntry already does have a BeanInfo ref, so adding method invoke is trivial.
It's the getClassloader (on InvokeDispatchContext) that is less obvious.
Probably securely getting it off the target is what we need to do.

I'll commit this during the day, but you should still be aware of this warning:
- http://www.jboss.org/index.html?module=bb&op=viewtopic&t=131997
:-)

[alesj]

"alesj" wrote:

Probably securely getting it off the target is what we need to do.

 // this already checks for permission
 return target.getClass().getClassLoader();

[adrian.brock]

"alesj" wrote:

"alesj" wrote:

Probably securely getting it off the target is what we need to do.

 // this already checks for permission
 return target.getClass().getClassLoader();

No. target.getClass().getClassLoader() is not correct (and is
pretty much always wrong). The TCL would be better, but not in the case.

The correct classloader (like all the other installs)
is to use the classloader of the bean doing the install.

Otherwise how could this work if you used the classloader where the KernelEventRegistry
is located (i.e. the bootstrap classloader/classpath)

<deployment xmlns="urn:jboss:bean-deployer:2.0">
 <classloader><!-- something unreachable from the bootstrap classloader --></classloader>
 <bean name="StartupMonitor"
 class="org.jboss.test.profileservice.lib.startup.StartupMonitor">
 <install bean="jboss.kernel:service=KernelEventManager" method="registerListener">
 <!-- registerListener(Object name, KernelEventListener listener, KernelEventFilter filter, Object han
dback) -->
 <parameter>jboss.kernel:service=KernelEventManager</parameter>
 <parameter><this/></parameter>
 <parameter><null/></parameter>
 <parameter><javabean class="from.my.classloader"/></parameter>
 </install>
 </bean>
</deployment>

[adrian.brock]

P.S. This example also needs an unregister listener to avoid memory leaks
and allow reboots of the server without halting the JVM.

[alesj]

The correct classloader (like all the other installs)
is to use the classloader of the bean doing the install.

But that's not the case right now.
At least if the target is not the bean who's doing the installs.

Should we change the InvokeDispatchHelper::invoke, to take the classloader param?

[alesj]

No. target.getClass().getClassLoader() is not correct (and is
pretty much always wrong). The TCL would be better, but not in the case.

What's then the 3rd option?

[adrian.brock]

"alesj" wrote:

The correct classloader (like all the other installs)
is to use the classloader of the bean doing the install.

But that's not the case right now.
At least if the target is not the bean who's doing the installs.

Should we change the InvokeDispatchHelper::invoke, to take the classloader param?

No we discussed this before
http://www.jboss.com/index.html?module=bb&op=viewtopic&t=128235
install/incallback use the classloader of what is being deployed.

This simulates exactly the bean doing it themselves in create/start()

If you know where this is not the case then it needs testing and fixing.