• Bug
  • Status: Resolved
  • 2 Major
  • Resolution: Fixed
  • ehcache-core
  • ljacomet
  • Reporter: candrews
  • October 28, 2013
  • 0
  • Watchers: 4
  • February 20, 2014
  • December 04, 2013

Description

When ehcache is shutting down, it tries to unregister all the JMX mbeans it has registered. When this fails, it logs an error, see http://grepcode.com/file/repo1.maven.org/maven2/net.sf.ehcache/ehcache/2.7.2/net/sf/ehcache/management/ManagementService.java#412

mbean unregistering can fail for a number of reasons, an especially common reason is that the bean has already been unregistered.

registerCacheStores already has an “isRegistered” guard - I think it would be great if that guard could also be used for the other mbeans.

I suggest that the current code, which looks like this:

    public void notifyCacheRemoved(String cacheName) {

        ObjectName objectName = null;
        try {
            if (registerCaches) {
                objectName = Cache.createObjectName(backingCacheManager.toString(), cacheName);
                mBeanServer.unregisterMBean(objectName);
            }
            if (registerCacheConfigurations) {
                objectName = CacheConfiguration.createObjectName(backingCacheManager.toString(), cacheName);
                mBeanServer.unregisterMBean(objectName);
            }
            if (registerCacheStatistics) {
                objectName = CacheStatistics.createObjectName(backingCacheManager.toString(), cacheName);
                mBeanServer.unregisterMBean(objectName);
            }
            if (registerCacheStores) {
                objectName = Store.createObjectName(backingCacheManager.toString(), cacheName);
                if (mBeanServer.isRegistered(objectName)) {
                    mBeanServer.unregisterMBean(objectName);
                }
            }
        } catch (Exception e) {
            LOG.error("Error unregistering cache for management for " + objectName
                    + " . Error was " + e.getMessage(), e);
        }

    }

be changed to this:

    public void notifyCacheRemoved(String cacheName) {

        ObjectName objectName = null;
        try {
            if (registerCaches) {
                objectName = Cache.createObjectName(backingCacheManager.toString(), cacheName);
                if (mBeanServer.isRegistered(objectName)) {
                    mBeanServer.unregisterMBean(objectName);
                }
            }
            if (registerCacheConfigurations) {
                objectName = CacheConfiguration.createObjectName(backingCacheManager.toString(), cacheName);
                if (mBeanServer.isRegistered(objectName)) {
                    mBeanServer.unregisterMBean(objectName);
                }
            }
            if (registerCacheStatistics) {
                objectName = CacheStatistics.createObjectName(backingCacheManager.toString(), cacheName);
                if (mBeanServer.isRegistered(objectName)) {
                    mBeanServer.unregisterMBean(objectName);
                }
            }
            if (registerCacheStores) {
                objectName = Store.createObjectName(backingCacheManager.toString(), cacheName);
                if (mBeanServer.isRegistered(objectName)) {
                    mBeanServer.unregisterMBean(objectName);
                }
            }
        } catch (Exception e) {
            LOG.error("Error unregistering cache for management for " + objectName
                    + " . Error was " + e.getMessage(), e);
        }

    }

Comments

Louis Jacomet Jacomet 2013-11-28

This looks like a sound suggestion. However, given that there is a proposed solution, how does the process work? If I remember correctly, we need to make him sign a contributor agreement before I can implement this, right?

James House 2013-12-02

Craig,

Could you please fill-out and submit a Contributor Agreement, which can be found here:

http://www.terracotta.org/confluence/display/release/How+to+Become+a+Terracotta+Contributor

Our devs could easily just do this work, but it would likely end up looking almost exactly like what you’re submitting anyway…

Craig Andrews 2013-12-03

I have emailed the contributor agreement - I believe everything should be all set.

Craig Andrews 2013-12-04

[~ljacomet] what version of ehcache will this fix be included in? Thank you!

Louis Jacomet Jacomet 2013-12-04

It will be in the upcoming 2.8.0 version, I will however merge the fix to the 2.7.x branch so that if a release happens there, it is included too.