groovy

GroovyMBean does not support the use of overloaded MBean operations that have the same number of parameters

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: JMX
  • Labels:
    None
  • Number of attachments :
    1

Description

I've been using groovy.util.GroovyMBean to access MBeans running on the JBoss 4 server. Some of the operations/methods of the MBeans are overloaded, e.g.:

void redeploy(java.lang.String url)
void redeploy(java.net.URL url)
void redeploy(org.jboss.deployment.DeploymentInfo sdi)

Currently it's impossible to invoke the first 2.

The problem is, when the GroovyMBean is constructed it stores a signature for each operation from its specified MBean in a HashMap, using the operation name and the number of parameters as its key. Later, when the invokeMethod method is invoked, it uses the method name and the number of parameters to retrieve the signature to invoke the operation on the MBean. Thus for overloaded operations with the same number of parameters, only the last operation found will be stored in the HashMap.

If another variant of the operation is invoked on the GroovyMBean then an incorrect signature will be passed to the MBeanServer which results in an exception occurring on the server.

The fix I've used (suggested by a comment in the source code) is to construct a key out of the name of the operation and the sum of the hash codes of the class names of the parameters.

The problem is, this required 2 new createOperationKey methods:

String createOperationKey(MBeanOperationInfo opInfo)
String createOperationKey(String method, Object[] args)

The first is invoked by the constructor, and the second that is invoked by the invokeMethod method. The original createOperationKey method is removed. This might create backwards compatibility problems.

I've included an adjusted GroovyMBean class, although temporarily in a new package (groovy.util.adjusted).

This issue is an extension of issue GROOVY-1291

Issue Links

supercedes

Bug - A problem which impairs or prevents the functions of the product. GROOVY-1291 GroovyMBean doesn't support the use of overloaded MBean operations

  • Critical - Crashes, loss of data, severe memory leak.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Meyrick Kirby added a comment -

Ah, I see the problem now. Groovy has to go through all kinds of fun to work out which method to call on ordinary classes, as seen in MetaClassImpl and MetaClassHelper, and there is a similar problem here.

Show
Meyrick Kirby added a comment - Ah, I see the problem now. Groovy has to go through all kinds of fun to work out which method to call on ordinary classes, as seen in MetaClassImpl and MetaClassHelper, and there is a similar problem here.
Hide
Permalink
Ludovic Claude added a comment -

I have the same issue with the standard ThreadMXBean: it exposes several versions of getThreadInfo, and Groovy picks a random choice for the method with one argument, and it's not possible to use a workaround with 2 arguments or more here...
ThreadInfo getThreadInfo(long id)
ThreadInfo[] getThreadInfo(long[] ids)
ThreadInfo[] getThreadInfo(long[] ids, int maxDepth)
ThreadInfo getThreadInfo(long id, int maxDepth)

Show
Ludovic Claude added a comment - I have the same issue with the standard ThreadMXBean: it exposes several versions of getThreadInfo, and Groovy picks a random choice for the method with one argument, and it's not possible to use a workaround with 2 arguments or more here... ThreadInfo getThreadInfo(long id) ThreadInfo[] getThreadInfo(long[] ids) ThreadInfo[] getThreadInfo(long[] ids, int maxDepth) ThreadInfo getThreadInfo(long id, int maxDepth)
Hide
Permalink
Guillaume Laforge added a comment -

Have you had a look at the JMX builder to see if it covers the same ground there?
Do you have some ideas on how to improve GroovyMBean to support this? (ie. a patch

Show
Guillaume Laforge added a comment - Have you had a look at the JMX builder to see if it covers the same ground there? Do you have some ideas on how to improve GroovyMBean to support this? (ie. a patch
Hide
Permalink
Paul King added a comment -

There are several issues with the supplied patch.

Firstly, the uniqueness of the hashing algorithm could be improved; e.g. if we had two operations:

method(String arg1, int arg2)
method(int arg1, String arg2)

The proposed algorithm (like the current one based on number of args) would produce the same key for each. This can be avoided by accumulating a hash multiplying by a prime for instance for each additional argument type.

However, the other problem is that it requires exactly matching type signatures, so for the following signature:

int plus(int arg1, int arg2)

It must be called with 2 integers. If called with (for instance) an integer and a short (which is allowed by Java, Groovy and the MBean spec) it wouldn't find the matching method.

I am still pondering whether something better can be done.

Show
Paul King added a comment - There are several issues with the supplied patch. Firstly, the uniqueness of the hashing algorithm could be improved; e.g. if we had two operations: method( String arg1, int arg2) method( int arg1, String arg2) The proposed algorithm (like the current one based on number of args) would produce the same key for each. This can be avoided by accumulating a hash multiplying by a prime for instance for each additional argument type. However, the other problem is that it requires exactly matching type signatures, so for the following signature: int plus( int arg1, int arg2) It must be called with 2 integers. If called with (for instance) an integer and a short (which is allowed by Java, Groovy and the MBean spec) it wouldn't find the matching method. I am still pondering whether something better can be done.
Hide
Permalink
Paul King added a comment -

For future reference, an improvement to the patch which fixes the first but not the second issue:

    /* *
     * Construct a key based on the operation/method name and a hash of the parameter types.
     *
     * It is important that this method returns the same key as the
     * <code>createOperationKey(String method, Object[] args)</code) method
     * for the equivalent input
     *
     * @param opInfo - the mbean operation/method information
     * @return unique identifier for an operation/method
     */
    protected String createOperationKey(MBeanOperationInfo opInfo) {
        int hashCode = 0;
        for (MBeanParameterInfo paramInfo : opInfo.getSignature()) {
            hashCode = hashCode + 17 * paramInfo.getType().hashCode();
        }
        return opInfo.getName() + "_" + hashCode;
    }

    /* *
     * Construct a key based on the operation/method name and a hash of the parameter types
     *
     * It is important that this method returns the same key as the
     * <code>createOperationKey(MBeanOperationInfo opInfo)</code) method
     * for the equivalent input
     *
     * @param method - the mbean operation/method name
     * @param args   - an array of the arguments passed to the operation/method
     * @return unique identifier for an operation/method
     */
    protected String createOperationKey(String method, Object[] args) {
        int hashCode = 0;
        for (Object arg : args) {
            hashCode = hashCode + 17 * arg.getClass().getName().hashCode();
        }
        return method + "_" + hashCode;
    }
Show
Paul King added a comment - For future reference, an improvement to the patch which fixes the first but not the second issue: /* * * Construct a key based on the operation/method name and a hash of the parameter types. * * It is important that this method returns the same key as the * <code>createOperationKey( String method, Object [] args)</code) method * for the equivalent input * * @param opInfo - the mbean operation/method information * @ return unique identifier for an operation/method */ protected String createOperationKey(MBeanOperationInfo opInfo) { int hashCode = 0; for (MBeanParameterInfo paramInfo : opInfo.getSignature()) { hashCode = hashCode + 17 * paramInfo.getType().hashCode(); } return opInfo.getName() + "_" + hashCode; } /* * * Construct a key based on the operation/method name and a hash of the parameter types * * It is important that this method returns the same key as the * <code>createOperationKey(MBeanOperationInfo opInfo)</code) method * for the equivalent input * * @param method - the mbean operation/method name * @param args - an array of the arguments passed to the operation/method * @ return unique identifier for an operation/method */ protected String createOperationKey( String method, Object [] args) { int hashCode = 0; for ( Object arg : args) { hashCode = hashCode + 17 * arg.getClass().getName().hashCode(); } return method + "_" + hashCode; }

People

  • Assignee:
    Unassigned
    Reporter:
    Meyrick Kirby
Vote (4)
Watch (4)

Dates

  • Created:
    Updated: