On Thu 06 Jun 2013 06:06:52 PM CEST, shanliang wrote: > Jaroslav Bachorik wrote: >> On Thu 06 Jun 2013 05:22:31 PM CEST, shanliang wrote: >> >>> Jaroslav, >>> >>> It is now OK for me about the MBean interface searching in the >>> Introspector. >>> >>> Here is my comment on JMX.java: >>> 206 -- 212 you added a call >>> Introspector.testComplianceMBeanInterface(interfaceClass); >>> >>> It is better to move this call to: >>> MBeanServerInvocationHandler.newProxyInstance >>> because the real job is done in newProxyInstance and it could be >>> directly called by anyone. >>> >> >> Hm, wouldn't it be better to move the actual logic from >> MBeanServerInvocationHandler.newProxyInstance to JMX.newMBeanProxy and >> delegate the MBSIH.newProxyInstance back to JMX.newMBeanProxy ? This >> way it would be consistent with the way JMX.newMXBeanProxy is written. >> > I have no opinion here, this is an implementation detail, anyway we > have to keep both JMX.newMBeanProxy and > MBeanServerInvocationHandler.newProxyInstance
http://cr.openjdk.java.net/~jbachorik/8010285/webrev.04 I've moved the newMBeanProxy() logic to JMX to keep it consistent with JMX.newMXBeanProxy(); MBSIH.newProxyInstance() is just forwarded to JMX.newMBeanProxy() -JB- > > Shanliang >> >>> All others are OK for me. >>> >> >> Thanks. >> >> -JB- >> >> >>> Shanliang >>> >>> Jaroslav Bachorik wrote: >>> >>>> On Wed 05 Jun 2013 07:54:10 PM CEST, shanliang wrote: >>>> >>>> >>>>> Daniel Fuchs wrote: >>>>> >>>>> >>>>>> On 6/5/13 3:55 PM, Jaroslav Bachorik wrote: >>>>>> >>>>>> >>>>>>>> class A extends B { ...} >>>>>>>> >>>>>>>> >>>>>>>>> class B implements AMBean {...} >>>>>>>>> >>>>>>>>> >>>>>>> Yes, I see it now. However, when you check the JMX specification, page >>>>>>> 50 onwards, the current implementation does not seem to be correct. >>>>>>> >>>>>>> "3. If MyClass is an instance of the DynamicMBean interface, then >>>>>>> MyClassMBean is >>>>>>> ignored. If MyClassMBean is not a public interface, it is not a JMX >>>>>>> manageable >>>>>>> resource. If the MBean is an instance of neither MyClassMBean nor >>>>>>> DynamicMBean, the inheritance tree of MyClass is examined, looking >>>>>>> for the >>>>>>> nearest superclass that implements its own MBean interface. >>>>>>> a. If there is an ancestor called SuperClass that is an instance of >>>>>>> SuperClassMBean, the design patterns are used to derive the >>>>>>> attributes and >>>>>>> operations from SuperClassMBean. In this case, the MBean MyClass then >>>>>>> has the same management interface as the MBean SuperClass. If >>>>>>> SuperClassMBean is not a public interface, it is not a JMX manageable >>>>>>> resource. >>>>>>> b. When there is no superclass with its own MBean interface, MyClass is >>>>>>> not a >>>>>>> Standard MBean." >>>>>>> >>>>>>> According to the specification the correct MBean interface for >>>>>>> >>>>>>> class A extends B { ...} >>>>>>> class B implements BMBean, AMBean >>>>>>> >>>>>>> would be BMBean >>>>>>> >>>>>>> >>>>>> Hi Jaroslav, >>>>>> >>>>>> Given that A is an instance of AMBean I think that according to the >>>>>> specification the correct interface should be AMBean. >>>>>> It's true that the JMX Specification does not explicitly speak of this >>>>>> case - but neither does it forbid it. >>>>>> >>>>>> My advice would therefore be to clarify the spec on this point, >>>>>> if that's needed - rather than risking the introduction of >>>>>> incompatibilities. >>>>>> >>>>>> -- daniel >>>>>> >>>>>> >>>>> Look at the spec 1.4: >>>>> ------ >>>>> 2. If the MyClass MBean is an instance of a MyClassMBean interface, >>>>> then only the methods listed in, or inherited by, the interface are >>>>> considered among all the methods of, or inherited by, the MBean. The >>>>> design patterns are then used to identify the attributes and >>>>> operations from the method names in the MyClassMBean interface and its >>>>> ancestors. In other words, MyClass is a standard MBean >>>>> ------ >>>>> >>>>> Here A is an instance of AMBean, according to 2), A is a standard >>>>> MBean and AMBean must be taken, >>>>> >>>>> 3) specifies the condition as "If the MBean is an instance of neither >>>>> MyClassMBean nor DynamicMBean", our example is out of this condition, >>>>> so should not apply 3) to our example. >>>>> >>>>> >>>> Ok. I've reverted to the original implementation. >>>> >>>> http://cr.openjdk.java.net/~jbachorik/8010285/webrev.03/ >>>> >>>> The whole MBean interface inferring algorithm is a bit of black magic, >>>> though. I mean, checking all the interfaces implemented by all the >>>> classes in the inheritance hierarchy is counter-intuitive. I mean, why >>>> would you do something like : >>>> MyServiceIfc extends ObscureIfc extends ServiceMBean >>>> Service implements MyServiceIfc >>>> >>>> and silently supposing that somewhere in the interface inheritance >>>> hierarchy just happen to be a properly named interface and my Service >>>> would become a managed resource. Not mentioning the fact, that the >>>> current implementation will fail to resolve the ServiceMBean as the >>>> MBean interface - it stops checking by ObscureIfc. >>>> >>>> It would be much easier for the user if he just specified the MBean >>>> interface alongside the implementation class (... implements ..., >>>> ServiceMBean) cleanly indicating that an object is a managed resource. >>>> >>>> But, what you gonna do ... changing the spec would probably open a >>>> whole another can of worms and since nobody is complaining about the >>>> current implementation we can just leave it as it is. >>>> >>>> -JB- >>>> >>>> >>>> >>>>> Shanliang >>>>> >>>>> >>>>> >>>>>>> and for >>>>>>> class A extends B { ...} >>>>>>> class B implements AMBean {...} >>>>>>> >>>>>>> is not defined; neither B or A are manageable resources. >>>>>>> >>>>>>> As I said, the jtreg and jck test does not seem to mind which >>>>>>> implementation is used, they all pass happily. I would prefer bringing >>>>>>> the implementation in sync with the specification. >>>>>>> >>>>>>> -JB- >>>>>>> >>>>>>> >>>>>>> >>>> >>>> >> >> >> >
