[ mx4j-Bugs-1031741 ] Caching Support In Required Model MBean Is Broken

Bugs item #1031741, was opened at 2004-09-21 10:41
Message generated for change (Comment added) made by emcmanus
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=450647&aid=1031741&group_id=47745

Category: JMX implementation
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Nobody/Anonymous (nobody)
Assigned to: Nobody/Anonymous (nobody)
Summary: Caching Support In Required Model MBean Is Broken

Initial Comment:
The following class is a JUnit test case which
demonstrates that two RequiredModelMBeans which share
the same ModelMBeanInfo class also share the same
attribute cache which leads to incorrect behaviour when
caching is enabled [ attribute values are aliased
between MBean instances ].

Both tests should pass, but only testWithoutCache does.

For more information about this testcase, write to
[email protected].


/*
 * Created on 21/09/2004
 *
 */
package bugs;

import javax.management.Attribute;
import javax.management.AttributeNotFoundException;
import javax.management.Descriptor;
import javax.management.InstanceNotFoundException;
import javax.management.InvalidAttributeValueException;
import javax.management.MBeanException;
import javax.management.ReflectionException;
import javax.management.modelmbean.DescriptorSupport;
import
javax.management.modelmbean.InvalidTargetObjectTypeException;
import javax.management.modelmbean.ModelMBeanAttributeInfo;
import
javax.management.modelmbean.ModelMBeanConstructorInfo;
import javax.management.modelmbean.ModelMBeanInfo;
import javax.management.modelmbean.ModelMBeanInfoSupport;
import
javax.management.modelmbean.ModelMBeanNotificationInfo;
import javax.management.modelmbean.ModelMBeanOperationInfo;
import javax.management.modelmbean.RequiredModelMBean;

import junit.framework.TestCase;

/**
 * @author jseymour
 *
 */
public class CachingBug
	extends TestCase
{
    private String foo;
    public String getFoo()
    {
        return foo;
    }
    public void setFoo(String foo) {
        this.foo = foo;
    }
    
    public void testWithoutCache() throws Exception {    
        theTest(false); 
    }
    
    public void testWithCache() throws Exception {    
        theTest(true); 
    }
    /**
     * @throws MBeanException
     * @throws InstanceNotFoundException
     * @throws InvalidTargetObjectTypeException
     * @throws AttributeNotFoundException
     * @throws InvalidAttributeValueException
     * @throws ReflectionException
     */
    private void theTest(boolean withCache) throws
MBeanException, InstanceNotFoundException,
InvalidTargetObjectTypeException,
AttributeNotFoundException,
InvalidAttributeValueException, ReflectionException {
        Descriptor d = new DescriptorSupport();
        
        if (withCache) {
            d.setField("currencyTimeLimit", "60");
        }
        
        d.setField("name", "foo");
        d.setField("descriptorType", "attribute");
        d.setField("getMethod", "getFoo");
        d.setField("setMethod", "setFoo");
        
        ModelMBeanAttributeInfo fooInfo = new
ModelMBeanAttributeInfo("foo", "java.lang.String", "is
a foo", true, true, false, d);
        ModelMBeanInfoSupport mbi = new
ModelMBeanInfoSupport(CachingBug.class.getName(),
"pojo", new ModelMBeanAttributeInfo[] { fooInfo } , new
ModelMBeanConstructorInfo[] {}, new
ModelMBeanOperationInfo[] {}, new
ModelMBeanNotificationInfo[] {} );
        RequiredModelMBean rmb1 = new
RequiredModelMBean(mbi);
        RequiredModelMBean rmb2 = new
RequiredModelMBean((ModelMBeanInfo)mbi.clone());
        
        CachingBug bar1 = new CachingBug();
        CachingBug bar2 = new CachingBug();
        
        rmb1.setManagedResource(bar1, "ObjectReference");
        rmb2.setManagedResource(bar2, "ObjectReference");
        
        rmb1.setAttribute(new Attribute("foo","bar1"));
        rmb2.setAttribute(new Attribute("foo","bar2"));
        assertEquals("value of rmb1.foo", "bar1",
rmb1.getAttribute("foo")); 
        assertEquals("value of rmb2.foo", "bar2",
rmb2.getAttribute("foo"));
    }
}


----------------------------------------------------------------------

Comment By: Eamonn McManus (emcmanus)
Date: 2004-09-21 17:41

Message:
Logged In: YES 
user_id=770046

Further note: ModelMBeanInfoSupport.clone() does explicitly
say that it is a shallow clone (using text inherited from
MBeanInfo.clone()). However, the question still applies to
ModelMBeanAttributeInfo.clone(), which does not say whether
it clones its Descriptor.

----------------------------------------------------------------------

Comment By: Eamonn McManus (emcmanus)
Date: 2004-09-21 17:32

Message:
Logged In: YES 
user_id=770046

I didn't notice the clone.  It could be argued that
ModelMBeanInfoSupport.clone() (or equivalently the "copy
constructor" of that class) should also clone the individual
ModelMBeanAttributeInfo entries (etc) and that
ModelMBeanAttributeInfo.clone() (or its copy constructor)
should clone the descriptor.  Or, the spec could at least
say explicitly that these are *not* cloned so that you would
know that code like the code here would not work.
I have logged bug 5104947 against this, which will soon be
visible on bugs.sun.com.  We could address this in the next
version of the JMX spec.  My inclination would be to leave
the clone shallow as it is today in both the Reference
Implementation and MX4J, but add a different attribute
caching mechanism that does not depend on modifying the
descriptor. (Nothing else modifies the descriptor over the
life of its MBean.)

----------------------------------------------------------------------

Comment By: Jon Seymour (jonseymour)
Date: 2004-09-21 16:51

Message:
Logged In: YES 
user_id=1125728

The other thing worth noting about this bug/test case is
that the two mbeans actually don't share an identical
mbeaninfo - the 2nd mbean shares a clone of the first
mbean's mbeaninfo structure yet both  modelmbeans end up
sharing each other's attribute cache. This is ultimately
because ModelMBeanInfoSupport and DescriptorSupport don't
perform a deep enough clone.

The JMX specification may be quiet about dependencies
between cloned instances of metadata classes but it is not
clear to me that MX4J's chosen interpretation is
particularly useful.



----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2004-09-21 11:33

Message:
Logged In: NO 

Ok - I can see you are correct. Is there a reason why
ModelMBeanInfoSupport.clone() doesn't do a deep clone or at
least a clone that is deep enough to ensure that a cloned
ModelMBeanInfoSupport instance actually has its own
attribute cache?

----------------------------------------------------------------------

Comment By: Eamonn McManus (emcmanus)
Date: 2004-09-21 10:56

Message:
Logged In: YES 
user_id=770046

For better or worse, RequiredModelMBean is specified to
cache attribute values in the attribute Descriptor. If two
RequiredModelMBeans share the same ModelMBeanInfo, then they
share the same ModelMBeanAttributeInfos and therefore the
same Descriptors for their attributes. As a consequence, if
you enable attribute caching, you cannot share your
MBeanInfo with anyone else.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=450647&aid=1031741&group_id=47745


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php