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

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

Category: JMX implementation
Group: Release 2.0.1
Status: Closed
Resolution: Wont Fix
Priority: 5
Submitted By: Nobody/Anonymous (nobody)
Assigned to: Simone Bordet (biorn_steedom)
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: Jon Seymour (jonseymour)
Date: 2004-10-06 00:04

Message:
Logged In: YES 
user_id=1125728

The other point, of course, is that I wasn't trying to share
meta data - I was trying very hard to use distinct instances
of a model mbean info each ModelMBean I instantiated - by
separately cloning the instance of the info structure handed
to each ModelMBean. 

As it stands the implementation forces me to walk the entire
ModelMBeanInfo tree and clone each and every node found in
that tree manually. I can't use the copy constructors and
clone methods provided by the implementation to do this job
because they don't implement reasonable copy/clone semantics.

However, I accept this is broken as designed.

jon.

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

Comment By: Jon Seymour (jonseymour)
Date: 2004-10-05 23:55

Message:
Logged In: YES 
user_id=1125728

As its stands MX4J's implementation of copy constructors and
clone methods are currently inconsistent with the reference
implementation's implementation - i.e. they do different things.

A particular example is DescriptorSupprt.clone(). It merely
copies the Map reference rather than duplicating the Map
itself as in the R.I.'s case.

If you do nothing MX4J will remain incompatible with the R.I.

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

Comment By: Simone Bordet (biorn_steedom)
Date: 2004-10-05 23:39

Message:
Logged In: YES 
user_id=128193

What can I say ? The specification will not change until
Mustang, which is spring/summer 2006, almost 2 years.
In the meanwhile, everybody will learn that sharing metadata
in MMB is looking for troubles (I should say that using MMB
is looking for troubles), so when the bug will be fixed,
probably noone will use the modified behavior anyway.
I'm not going to fix (to keep compatibility with
JMXRI/JDK5.0), so I'm closing the bug as "won't fix".

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

Comment By: Jon Seymour (jonseymour)
Date: 2004-09-22 12:28

Message:
Logged In: YES 
user_id=1125728

There certainly appears to be imprecision in the
specification about whether it is intended to be able to
customize metadata instances with instance specific
metadata. On the one hand the spec allows it (attribute
caches) but then doesn't provide sufficient detail about the
behaviour of clone() methods and copy constructors to enable
these to be usefully used.

BTW: There is a similar problem with modifying field values
of the ModelMBean descriptor itself. Different clones of the
ModelMBeanInfo instance share the same descriptor value
cache.  Again, this leads to counterintuitive results if it
is intended for different ModelMBeanInfo instances to have
different descriptor value maps.

This problem is a consequence of the other problem I
reported - the implementation of DescriptorSupport.clone()
being too shallow.

There is also a problem with cached results of operations,
though why one would want to create an operation that caches
a result is somewhat beyond me.

So, I agree - I think the JMX specification needs work in
two areas: what assumptions can users make about the
independence or otherwise of metadata instance clones +
copies and where should instance-specific metadata (such as
caches)  be stored.

jon.

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

Comment By: Eamonn McManus (emcmanus)
Date: 2004-09-22 01: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-22 01: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-22 00: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 19: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 18: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: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl