Problem dispatching array as argument to a closure

Details

Type: Bug
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: 1.8.2
Fix Version/s: 2.1.0-rc-1, 1.8.9, 2.0.7
Component/s: None
Labels:
None

Number of attachments :
0

Description

myscript.groovy

ExpandoMetaClass.enableGlobally()

Object[] myObjectArray = ['a', 'b'] as Object[]

closure = {
	println 'closure running...'
}
	
closure(myObjectArray)

code $ groovy -version
Groovy Version: 1.8.2 JVM: 1.6.0_26
code $ groovy myscript.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: myscript$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: myscript$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at myscript.run(myscript.groovy:9)

I have tested with versions all the way back to 1.7.0 and the exception is thrown for all the versions I tested.

If I remove the ExpandoMetaClass.enableGlobally(), the exception goes away.

I think this is related to http://jira.grails.org/browse/GRAILS-8002, though I am not sure why that problem just showed up in Grails 2.0-M2. I have not been able to reproduce it with Grails 1.3.7.

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Roshan Dawrani added a comment - 12/Sep/11 10:31 PM

Looked a bit into it and obviously the problem is related to how ClosureMetaClass(CMC) and ExpandoMetaClass choose the method to invoke.

1) The ClosureMetaClass chooses the method to invoke via StandardClosureChooser as it finds the following two methods burnt into the class and restricts its method selection to these two going just by the number of arguments (and selects "doCall(Object)" when "closure(myObjectArray)" call is made:

- public Object doCall()
- public Object doCall(Object it)

2) On the other hand, the EMC makes its choice from among the following methods and chooses Closure.call(Object... args) as a more exact match:

- public Object groovy.lang.Closure.call()
- public Object groovy.lang.Closure.call(java.lang.Object[])
- public Object groovy.lang.Closure.call(java.lang.Object)

I think the difference in behavior is arising because groovy.lang.Closure has an extra "call(Object...)" and its counterpart "doCall(Object...)" is missing in closure's bytecode. Shouln't these two sets of methods be in sync always? Once they are, we should have consistent behavior one way or another, whether CMC is used, or EMC.

Show

Roshan Dawrani added a comment - 12/Sep/11 10:31 PM Looked a bit into it and obviously the problem is related to how ClosureMetaClass(CMC) and ExpandoMetaClass choose the method to invoke. 1) The ClosureMetaClass chooses the method to invoke via StandardClosureChooser as it finds the following two methods burnt into the class and restricts its method selection to these two going just by the number of arguments (and selects "doCall(Object)" when "closure(myObjectArray)" call is made: - public Object doCall() - public Object doCall(Object it) 2) On the other hand, the EMC makes its choice from among the following methods and chooses Closure.call(Object... args) as a more exact match: - public Object groovy.lang.Closure.call() - public Object groovy.lang.Closure.call(java.lang.Object[]) - public Object groovy.lang.Closure.call(java.lang.Object) I think the difference in behavior is arising because groovy.lang.Closure has an extra "call(Object...)" and its counterpart "doCall(Object...)" is missing in closure's bytecode. Shouln't these two sets of methods be in sync always? Once they are, we should have consistent behavior one way or another, whether CMC is used, or EMC.

Hide

Permalink

Roshan Dawrani added a comment - 12/Sep/11 11:12 PM

Apart from the above mentioned inconsistency on Groovy side, there is another inconsistency on Java side that is in play - related to how the direct dispatch in it differs from a reflection based dispatch:

import java.lang.reflect.Method;

public class TryInJava {
    public static void main(String[] args) throws Exception {
        Object[] strs = new Object[] {"a", "b"};
        Object[] inputs = new Object[] {strs};
        Method m = Foo.class.getMethod("call", Object[].class);
        Foo foo = new Foo();
        foo.call(inputs); // this prints "1" as the inputs array reaches method "call" wrapped properly - as {{"a", "b"}}.
        m.invoke(foo, inputs); // this prints "2" as now the inputs array reaches method "call" unwrapped - as {"a", "b"}.
    }
}


class Foo {
    public void call(Object[] inputs) {
        System.out.println(inputs.length);
    }
}

Since we cannot remove call(Object... args)/call(Object[]) from public API of groovy.lang.Closure and since Groovy has to do a reflection based call, removing the Groovy side inconsistency may result in failure to dispatch the array to a closure in both CMC and EMC cases - unless we then treat it as some special case.

Show

Roshan Dawrani added a comment - 12/Sep/11 11:12 PM Apart from the above mentioned inconsistency on Groovy side, there is another inconsistency on Java side that is in play - related to how the direct dispatch in it differs from a reflection based dispatch: import java.lang.reflect.Method; public class TryInJava { public static void main( String [] args) throws Exception { Object [] strs = new Object [] { "a" , "b" }; Object [] inputs = new Object [] {strs}; Method m = Foo.class.getMethod( "call" , Object [].class); Foo foo = new Foo(); foo.call(inputs); // this prints "1" as the inputs array reaches method "call" wrapped properly - as {{ "a" , "b" }}. m.invoke(foo, inputs); // this prints "2" as now the inputs array reaches method "call" unwrapped - as { "a" , "b" }. } } class Foo { public void call( Object [] inputs) { System .out.println(inputs.length); } } Since we cannot remove call(Object... args)/call(Object[]) from public API of groovy.lang.Closure and since Groovy has to do a reflection based call, removing the Groovy side inconsistency may result in failure to dispatch the array to a closure in both CMC and EMC cases - unless we then treat it as some special case.

Hide

Permalink

blackdrag blackdrag added a comment - 13/Sep/11 4:40 AM

I think the solution is to make EMC doCall instead of call

Show

blackdrag blackdrag added a comment - 13/Sep/11 4:40 AM I think the solution is to make EMC doCall instead of call

Hide

Permalink

blackdrag blackdrag added a comment - 13/Sep/11 4:41 AM

call is intended to be a kind of dispatcher method and actually behaving as it should, even though that makes no sense if called directly in Grovy

Show

blackdrag blackdrag added a comment - 13/Sep/11 4:41 AM call is intended to be a kind of dispatcher method and actually behaving as it should, even though that makes no sense if called directly in Grovy

Hide

Permalink

Graeme Rocher added a comment - 21/Sep/11 3:16 AM

Can we prioritize a fix on this? I would like to get it tested against Grails as I'm concerned about what other regressions could be caused by making a change in behavior like this. In particular I'm concerned about meta-programming logic in EMC and whether it will be impacted by this change.

For example what will be the impact on methodMissing logic like this which currently works:

https://github.com/SpringSource/grails-data-mapping/blob/44031e133d3f7d3c93f575c38b4038bee246292c/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy#L102

Anyway regardless I think it is important we get this change in soon so we can do some appropriate testing

Show

Graeme Rocher added a comment - 21/Sep/11 3:16 AM Can we prioritize a fix on this? I would like to get it tested against Grails as I'm concerned about what other regressions could be caused by making a change in behavior like this. In particular I'm concerned about meta-programming logic in EMC and whether it will be impacted by this change. For example what will be the impact on methodMissing logic like this which currently works: https://github.com/SpringSource/grails-data-mapping/blob/44031e133d3f7d3c93f575c38b4038bee246292c/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy#L102 Anyway regardless I think it is important we get this change in soon so we can do some appropriate testing

Hide

Permalink

blackdrag blackdrag added a comment - 21/Sep/11 8:59 AM

should be fixed now

Show

blackdrag blackdrag added a comment - 21/Sep/11 8:59 AM should be fixed now

Hide

Permalink

Graeme Rocher added a comment - 26/Sep/11 2:09 AM

This still seems to be an issue. Code to reproduce:

ExpandoMetaClass.enableGlobally()
def args = [1, 2] as Object[]
def c = {}
c(args)

Exception:

groovy.lang.MissingMethodException: No signature of method: ConsoleScript6$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:55)
	at groovy.lang.Closure$call.call(Unknown Source)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:42)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
	at ConsoleScript6.run(ConsoleScript6:4)
	at groovy.lang.GroovyShell.runScriptOrMainOrTestOrRunnable(GroovyShell.java:266)

Show

Graeme Rocher added a comment - 26/Sep/11 2:09 AM This still seems to be an issue. Code to reproduce: ExpandoMetaClass.enableGlobally() def args = [1, 2] as Object [] def c = {} c(args) Exception: groovy.lang.MissingMethodException: No signature of method: ConsoleScript6$_run_closure1.doCall() is applicable for argument types: (java.lang. Integer , java.lang. Integer ) values: [1, 2] Possible solutions: doCall(), doCall(java.lang. Object ), call(), call([Ljava.lang. Object ;), call(java.lang. Object ), findAll() at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:55) at groovy.lang.Closure$call.call(Unknown Source) at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:42) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116) at ConsoleScript6.run(ConsoleScript6:4) at groovy.lang.GroovyShell.runScriptOrMainOrTestOrRunnable(GroovyShell.java:266)

Hide

Permalink

Jeff Brown added a comment - 26/Sep/11 4:32 PM

This looks like it may be a regression introduced in 1.8.1.

doit.groovy

ExpandoMetaClass.enableGlobally()
List l = new ArrayList()
l << ([1, 2] as Object[])

l.each {
	println "it is ${it}"
}

With 1.8.0...

grv $ groovy -version
Groovy Version: 1.8.0 JVM: 1.6.0_26
grv $ groovy doit.groovy 
it is [1, 2]
grv $

With 1.8.1...

grv $ groovy -version
gGroovy Version: 1.8.1 JVM: 1.6.0_26
grv $ groovy doit.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at doit.run(doit.groovy:5)
grv $

With 1.8.2...

grv $ groovy -version
grGroovy Version: 1.8.2 JVM: 1.6.0_26
ogrv $ groovy doit.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at doit.run(doit.groovy:5)
grv $

Show

Jeff Brown added a comment - 26/Sep/11 4:32 PM This looks like it may be a regression introduced in 1.8.1. doit.groovy ExpandoMetaClass.enableGlobally() List l = new ArrayList() l << ([1, 2] as Object []) l.each { println "it is ${it}" } With 1.8.0... grv $ groovy -version Groovy Version: 1.8.0 JVM: 1.6.0_26 grv $ groovy doit.groovy it is [1, 2] grv $ With 1.8.1... grv $ groovy -version gGroovy Version: 1.8.1 JVM: 1.6.0_26 grv $ groovy doit.groovy Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() at doit.run(doit.groovy:5) grv $ With 1.8.2... grv $ groovy -version grGroovy Version: 1.8.2 JVM: 1.6.0_26 ogrv $ groovy doit.groovy Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() at doit.run(doit.groovy:5) grv $

Hide

Permalink

blackdrag blackdrag added a comment - 29/Sep/11 3:42 PM

the original example of this issue still holds, but is not ot be fixed for 1.8 or 1.7 and is not o be fixed for Grails as well

Show

blackdrag blackdrag added a comment - 29/Sep/11 3:42 PM the original example of this issue still holds, but is not ot be fixed for 1.8 or 1.7 and is not o be fixed for Grails as well

Hide

Permalink

Graeme Rocher added a comment - 15/Dec/11 7:36 AM

Any update on this issue?

Show

Graeme Rocher added a comment - 15/Dec/11 7:36 AM Any update on this issue?

Hide

Permalink

Graeme Rocher added a comment - 11/Jan/12 2:07 AM

Guys are there any plans to fix this?

Show

Graeme Rocher added a comment - 11/Jan/12 2:07 AM Guys are there any plans to fix this?

Hide

Permalink

blackdrag blackdrag added a comment - 11/Jan/12 3:43 AM - edited

The problem is how to fix this. We cannot fix this without breaking code at another place. The call(Object...) could be called with call('a','b') or with def x=['a','b'] as Object[]; call(x). For the call method they are the same, but for the doCall methods they are not!

For example... let us say we add a doCall(Object...) method, then either we would let it do what call does and in the end have the same problem and nothing fixed, or we change the type of the implicit it to be an Object[] instead of being an Object. The disadvantages of this are the semantic change for the much implicit it parameter, which will break a lot of code, and that you will then never get a MissingMethodException from the closure unless you gave in the type yourself. And even then it won't be always correct... assuming you have cl=

{a,b,->...}

, and then do cl('a','b') you expect this to work. So far so good... but cl(x), with x being the Object[] will still also call that one

The other possible way would be to do what ClosureMetaClass does and not even think about calling the call method, but instead directing to doCall. the result for me: 11 failures and 401 errors in the build.

Show

blackdrag blackdrag added a comment - 11/Jan/12 3:43 AM - edited The problem is how to fix this. We cannot fix this without breaking code at another place. The call(Object...) could be called with call('a','b') or with def x= ['a','b'] as Object[]; call(x). For the call method they are the same, but for the doCall methods they are not! For example... let us say we add a doCall(Object...) method, then either we would let it do what call does and in the end have the same problem and nothing fixed, or we change the type of the implicit it to be an Object[] instead of being an Object. The disadvantages of this are the semantic change for the much implicit it parameter, which will break a lot of code, and that you will then never get a MissingMethodException from the closure unless you gave in the type yourself. And even then it won't be always correct... assuming you have cl= {a,b,->...} , and then do cl('a','b') you expect this to work. So far so good... but cl(x), with x being the Object[] will still also call that one The other possible way would be to do what ClosureMetaClass does and not even think about calling the call method, but instead directing to doCall. the result for me: 11 failures and 401 errors in the build.

Hide

Permalink

blackdrag blackdrag added a comment - 18/Jan/13 9:40 AM

this is reported as fixed in 1.8.3

Show

blackdrag blackdrag added a comment - 18/Jan/13 9:40 AM this is reported as fixed in 1.8.3

Hide

Permalink

Jeff Brown added a comment - 18/Jan/13 9:49 AM

~ $ groovy -version
Groovy Version: 1.8.8 JVM: 1.6.0_37 Vendor: Apple Inc. OS: Mac OS X
~ $ 
~ $ cat doit.groovy 
ExpandoMetaClass.enableGlobally()

Object[] myObjectArray = ['a', 'b'] as Object[]

closure = {
    println 'closure running...'
}

closure(myObjectArray)

~ $ 
~ $ groovy doit.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at doit.run(doit.groovy:9)
~ $

Show

Jeff Brown added a comment - 18/Jan/13 9:49 AM ~ $ groovy -version Groovy Version: 1.8.8 JVM: 1.6.0_37 Vendor: Apple Inc. OS: Mac OS X ~ $ ~ $ cat doit.groovy ExpandoMetaClass.enableGlobally() Object[] myObjectArray = ['a', 'b'] as Object[] closure = { println 'closure running...' } closure(myObjectArray) ~ $ ~ $ groovy doit.groovy Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.String, java.lang.String) values: [a, b] Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll() at doit.run(doit.groovy:9) ~ $

Hide

Permalink

blackdrag blackdrag added a comment - 18/Jan/13 9:53 AM

reopen for now till I verified it is really fixed in 1.8.9

Show

blackdrag blackdrag added a comment - 18/Jan/13 9:53 AM reopen for now till I verified it is really fixed in 1.8.9

Hide

Permalink

Jeff Brown added a comment - 18/Jan/13 9:54 AM

Should we remove 1.8.3 from the "Fix Version" field?

Show

Jeff Brown added a comment - 18/Jan/13 9:54 AM Should we remove 1.8.3 from the "Fix Version" field?

Hide

Permalink

blackdrag blackdrag added a comment - 24/Jan/13 3:24 AM

looks like I made a fix, but then did not apply it to 2.0.x and 1.8.x

Show

blackdrag blackdrag added a comment - 24/Jan/13 3:24 AM looks like I made a fix, but then did not apply it to 2.0.x and 1.8.x

Hide

Permalink

blackdrag blackdrag added a comment - 24/Jan/13 7:31 AM

I found the fix I did back then and that it is indeed in master and 2.1.0-RC-x, but is missing on 1.8.x and 2.0.x. So I pushed the change there as well and close the issue with that

Show

blackdrag blackdrag added a comment - 24/Jan/13 7:31 AM I found the fix I did back then and that it is indeed in master and 2.1.0-RC-x, but is missing on 1.8.x and 2.0.x. So I pushed the change there as well and close the issue with that

People

Assignee:

blackdrag blackdrag

Reporter:

Jeff Brown

Vote (9)

Watch (9)

Dates

Created:

12/Sep/11 9:12 AM

Updated:

25/Jan/13 11:35 PM

Resolved:

24/Jan/13 7:31 AM