New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InstanceNotFoundException can be avoided in MBeanServerExecutorLocal #76

Closed
marosmars opened this Issue Mar 1, 2013 · 1 comment

Comments

Projects
None yet
2 participants
@marosmars
Contributor

marosmars commented Mar 1, 2013

We are using latest(1.1.0-SNAPSHOT) code from integration branch for jolokia-osgi module. It works properly but there is one minor issue.

The method "private MBeanServer lookupJolokiaMBeanServer()" uses InstanceNotFoundException thrown by MBeanServer to check if the "jolokia:type=MBeanServer" bean is already registered. Our problem is that we log all exceptions thrown by MBeanServer and the InstanceNotFoundException from lookupJolokiaMBeanServer is logged by our logger. We would like to avoid it.

There is a simple solution for this and insted of relying on InstanceNotFoundException the check in lookupJolokiaMBeanServer() can be performed by utilising MBeanServer.isRegistered(...) method.

The fix would be :

// Check, whether the Jolokia MBean Server is available. If not, register
// at the Platform MBeanServer delegate to get notified when it gets registered
private MBeanServer lookupJolokiaMBeanServer() {
    MBeanServer server = ManagementFactory.getPlatformMBeanServer();
    ObjectName holderMBeanName = null;
    try {
        holderMBeanName = new ObjectName("jolokia:type=MBeanServer");
        if(server.isRegistered(holderMBeanName))
            return (MBeanServer) server.getAttribute(holderMBeanName,"JolokiaMBeanServer");
        else {
            // Not yet available. Register for when it comes been available.
            MBeanServerNotificationFilter filter = new MBeanServerNotificationFilter();
            filter.enableObjectName(holderMBeanName);
            try {
                server.addNotificationListener(getMBeanServerDelegateName(), this, filter, null);
            } catch (InstanceNotFoundException e) {
                // Will not happen, since a delegate is always created during the creation
                // of an MBeanServer
                throw new IllegalStateException("Internal: Cannot lookup " +
                                                getMBeanServerDelegateName() + ": " + e,e);
            } catch (NoSuchFieldError error) {
                // Might happen e.g. on Java 1.5, which doesn't know about MBeanServerDelegate.DELEGATE_NAME ...
            }
            return null;
        }
    } catch (JMException e) {
        throw new IllegalStateException("Internal: Cannot get Jolokia MBeanServer via JMX lookup: " + e,e);
    }
}
@rhuss

This comment has been minimized.

Owner

rhuss commented Mar 1, 2013

I can see your point (and yes, I know about server.isRegistered()). My initial point here was that most of the time this call should succeed, so the intention was to save a call. However, since this method is only called once is perfectly save to check beforehand whether the MBean is registered. I will add the check for 1.1.1 (1.1.0 was just released ;-(

@ghost ghost assigned rhuss Mar 1, 2013

rhuss added a commit that referenced this issue Mar 1, 2013

Check whether the Jolokia MBeanServerHolder is registered in advance …
…instead of relying on an InstanceNotFoundException (#76)

@rhuss rhuss closed this Mar 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment