On Wed 29 May 2013 07:44:34 PM CEST, Daniel Fuchs wrote: > On 5/29/13 7:17 PM, Jaroslav Bachorik wrote: >> On Wed 29 May 2013 05:33:21 PM CEST, Eamonn McManus wrote: >>> I would recommend against changing the code to do additional calls to >>> Class.forName during MBean introspection. As I recall we made the >>> opposite change some years ago, both because Class.forName can be slow >>> (it may call out to a user ClassLoader) and because it is a potential >>> source of security problems. >> >> Thanks. I was trying to dig some history from mercurial but couldn't. >> Walking through all the related interfaces is equally acceptable - I've >> tried both of the solutions and they test well with the regtests. >> >> I am still puzzled by the current implementation which will fail to >> locate the correct MBean interface in eg. >> >> <<CInterface>> extends <<BInterface>> extends <<ServiceMBean>> >> >> ClassA extends Service implements <<CInterface>> >> >> as the process would stop on <<BInterface>> (checks the superclass of >> the ClassA, checks all the interfaces implemented by the Service class, >> checks all the interfaces extended by <<CInterface>>) which plainly >> does not conform to the MBean interface naming convention and would >> miss the <<ServiceMBean>> interface. > > Hi Jaroslav, > > <<Service>> would have to implement <<ServiceMBean>> either > directly or indirectly. > > So the current implementation is correct. > > If <<ServiceMBean>> is not assignable from <<Service>> then > <<ServiceMBean>> is not an MBean interface for ClassA.
Actually, when you do ClassA extends Service implements <<BInterface>> the Introspector will return <<ServiceMBean>> as the standard mbean interface for ClassA. I've just tried it on a simple project to make sure I understand the code correctly. The puzzle is which behaviour is correct? Either all the levels of the interface hierarchy should be checked for the [className]MBean interfaces or none, I guess. However, I can not find anything in the spec related to this case. -JB- > > You can work around that by wrapping an instance of ClassA > in an instance of javax.management.StandardMBean, and by > specifying <<ServiceMBean>>.class as the MBean interface > in the constructor. > > Hope this helps, > > -- daniel > >> >> -JB- >> >>> >>> Éamonn >>> >>> >>> 2013/5/29 Jaroslav Bachorik <jaroslav.bacho...@oracle.com >>> <mailto:jaroslav.bacho...@oracle.com>> >>> >>> Updated webrev - >>> http://cr.openjdk.java.net/~jbachorik/8010285/webrev.01 >>> >>> It adds regtests and takes care of the comments from David and >>> Shanliang. >>> >>> -JB- >>> >>> On 05/28/2013 04:22 PM, Jaroslav Bachorik wrote: >>> > The fix enforces the management interfaces (read MBean and >>> MXBean >>> > interfaces) being public. While this is defined in the >>> specification it >>> > was not enforced in any way and it was allowed to create MBeans >>> for eg. >>> > private MBean interfaces. >>> > >>> > The fix adds checks when creating and registering MBeans and >>> throws >>> > javax.management.NotCompliantMBeanException when a user tries to >>> create >>> > an MBean with non-public management interface. >>> > >>> > Since this change can cause problems for users having non-public >>> > management interfaces a system property is introduced that will >>> revert >>> > to the old behaviour when set >>> (com.sun.jmx.mbeans.allowNonPublic). >>> > >>> > Thanks, >>> > >>> > -JB- >>> > >>> >>> >> >> >