*Info classes internal array structures

[squirest]

Sheesh, I figured implmenting MBeanInfo.clone() would be pure trivia. Instead it's opened a can of worms in my head.

It all boils down to trust. Do we trust users of JBossJMX to not muck around with the arrays passed in as constructor args to the various *Info classes? Do we equally trust users not to muck around with the [] return values from getXXX() methods?

All (?) of the *Info classes are immutable - in that they have no setXXX() methods. However, they *can* be tampered with by changing the object reference at a given position in a returned or supplied array.

In the current codebase this sort of tampering doesn't cause the mbeanserver to do anything evil like dispatch a method call to a different method. But that doesn't mean a future change won't make that possible (though probably unlikely).

To me it's not an easy decision to make. If we protect the *Info classes for one scenario we *need* to protect them for *all* scenarios. This means cloning all inputs (i.e. in constructors) and cloning all outputs (i.e. in getXXX() methods).

For the vast majority of cases all that cloning would be a real waste.

Sigh. Yet another (trivial) example of the sort of problem you get when the API is a little too much like an implementation.

So, trust or be paranoid? Guidance appreciated.

Trevor

[juha]

I don't there's a problem in making a copy in the MBeanInfo constructor. It is built once and lives through the whole lifecycle of an MBean, so the cost in constructor is not that critical.

The accessor methods are similar in this regard. Since the management interface of an MBean is constant, I'd expect most client programs to retrieve the management interface description only once, or rarely.

However, the Model MBean metadata classes implement the DescriptorAccess interface, which allows the descriptors to be changed, in other words, the Model MBean info is not completely immutable. We need to force the setDescriptor() methods to go through the interceptor stack in order to implement ACLs for different descriptors. Hmm.. need to think about that some more.

[squirest]

I agree MMB and DescriptorAccess important - though I'm not sure it's too bad 'cause the *Infos are still immutable.

I think the spec says somewhere about that while the ModelMBean *can* change its capabilities (*Info stuff) during its lifetime, doing so isn't guaranteed to work with all mgmt interfaces (or something like that). Think of a servlet HTML console which caches the *Infos. Heck *you* even mention it in your mail when you say "I'd expect most client programs to retrieve the management interface description only once, or rarely".

As far as the ModelMBean and DescriptorAccess guff is concerned, the MMB should just do what it's told whenever (imho).

The thing that gives off the greatest smell is the damn ModelMBeanInfoSupport class - it extends MBeanInfo *and* it has a copy constructor. Ewwww.

It was that hierarchy that got me thinking about being ultra-paraniod and cloning everything in and everything out.

Trev

[juha]

yes go ahead and clone

[squirest]

*Info classes with array members are done, will be checking in soon.

Moving on to ObjectName, got an idea to keep it quick while still immutable. Will add support for patterns at the same time.

That ok?
Trev

[juha]

yes, go ahead

ObjectName.equals() seems to have the most effect performance wise in a HashMap based registry, so if you can figure out a way to speed that up, you will likely see the result directly in invocation times.

[squirest]

I was going to attack it on two levels.

First, precompute (during construction) the return value for ObjectName.hashCode() based on the hashCode() of the canonical name string. This is safe 'cause it's immutable and it should make equality searches faster (right?)

Second, I was going to create a read-only subclass of java.util.HashTable so that getKeyPropertyList() didn't have to keep returning copy-ctor'd HashTables. This should make thirt-party (i.e. MBeanServer) pattern comparisons less costly.

If you spot flaws in my logic pls let me know.

Trev