This is an automated email from the ASF dual-hosted git repository. sunlan pushed a commit to branch GROOVY-7785 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e998ac3cf5c5795d5153fb35fa1c3a484ee75ade Author: Daniel Sun <[email protected]> AuthorDate: Mon Feb 2 01:38:40 2026 +0900 GROOVY-7785: StackoverflowException when using too many chained method calls --- .../classgen/asm/indy/InvokeDynamicWriter.java | 136 ++++++++++++++++- src/test/groovy/bugs/Groovy7785.groovy | 164 ++++++++++++++++++++- 2 files changed, 290 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/indy/InvokeDynamicWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/indy/InvokeDynamicWriter.java index 059f20bfc5..21e8204bdf 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/indy/InvokeDynamicWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/indy/InvokeDynamicWriter.java @@ -25,6 +25,7 @@ import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.ConstructorCallExpression; import org.codehaus.groovy.ast.expr.EmptyExpression; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.MethodCallExpression; import org.codehaus.groovy.ast.expr.PropertyExpression; import org.codehaus.groovy.ast.tools.WideningCategories; import org.codehaus.groovy.classgen.AsmClassGenerator; @@ -42,8 +43,10 @@ import org.objectweb.asm.Opcodes; import java.lang.invoke.CallSite; import java.lang.invoke.MethodHandles.Lookup; import java.lang.invoke.MethodType; +import java.util.ArrayList; import java.util.List; +import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression; import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression; import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE; import static org.codehaus.groovy.ast.ClassHelper.boolean_TYPE; @@ -54,16 +57,16 @@ import static org.codehaus.groovy.ast.ClassHelper.isWrapperBoolean; import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX; import static org.codehaus.groovy.classgen.asm.BytecodeHelper.doCast; import static org.codehaus.groovy.classgen.asm.BytecodeHelper.getTypeDescription; -import static org.codehaus.groovy.vmplugin.v8.IndyInterface.GROOVY_OBJECT; -import static org.codehaus.groovy.vmplugin.v8.IndyInterface.IMPLICIT_THIS; -import static org.codehaus.groovy.vmplugin.v8.IndyInterface.SAFE_NAVIGATION; -import static org.codehaus.groovy.vmplugin.v8.IndyInterface.SPREAD_CALL; -import static org.codehaus.groovy.vmplugin.v8.IndyInterface.THIS_CALL; import static org.codehaus.groovy.vmplugin.v8.IndyInterface.CallType.CAST; import static org.codehaus.groovy.vmplugin.v8.IndyInterface.CallType.GET; import static org.codehaus.groovy.vmplugin.v8.IndyInterface.CallType.INIT; import static org.codehaus.groovy.vmplugin.v8.IndyInterface.CallType.INTERFACE; import static org.codehaus.groovy.vmplugin.v8.IndyInterface.CallType.METHOD; +import static org.codehaus.groovy.vmplugin.v8.IndyInterface.GROOVY_OBJECT; +import static org.codehaus.groovy.vmplugin.v8.IndyInterface.IMPLICIT_THIS; +import static org.codehaus.groovy.vmplugin.v8.IndyInterface.SAFE_NAVIGATION; +import static org.codehaus.groovy.vmplugin.v8.IndyInterface.SPREAD_CALL; +import static org.codehaus.groovy.vmplugin.v8.IndyInterface.THIS_CALL; import static org.objectweb.asm.Opcodes.H_INVOKESTATIC; import static org.objectweb.asm.Opcodes.IFNULL; @@ -113,12 +116,133 @@ public class InvokeDynamicWriter extends InvocationWriter { // load normal receiver as first argument compileStack.pushImplicitThis(implicitThis); - receiver.visit(controller.getAcg()); + // GROOVY-7785: use iterative approach to avoid stack overflow for chained method calls + visitReceiverOfMethodCall(receiver); compileStack.popImplicitThis(); return "(" + getTypeDescription(operandStack.getTopOperand()); } + /** + * Visit receiver expression iteratively to avoid stack overflow for deeply nested method call chains. + * For chained calls like a().b().c()...z(), the AST forms a deep right-recursive structure where + * each method call's receiver is another method call. This method flattens the chain and processes + * it iteratively from the innermost receiver outward. + */ + private void visitReceiverOfMethodCall(final Expression receiver) { + // Collect the chain of method calls that can be handled by indy + List<MethodCallExpression> chain = new ArrayList<>(); + Expression current = receiver; + while (current instanceof MethodCallExpression mce && canUseIndyForChain(mce)) { + chain.add(mce); + current = mce.getObjectExpression(); + } + + if (chain.isEmpty()) { + // Not a chainable method call or chain cannot be optimized, use normal visit + receiver.visit(controller.getAcg()); + return; + } + + // Process the innermost non-chainable receiver first + current.visit(controller.getAcg()); + + // Process each method call in the chain, from innermost to outermost + AsmClassGenerator acg = controller.getAcg(); + for (int i = chain.size() - 1; i >= 0; i -= 1) { + MethodCallExpression mce = chain.get(i); + acg.onLineNumber(mce, "visitMethodCallExpression (chained): \"" + mce.getMethod() + "\":"); + // Process this method call with its receiver already on the stack + makeIndyCallWithReceiverOnStack(mce); + controller.getAssertionWriter().record(mce.getMethod()); + } + } + + /** + * Check if a method call can be handled in the chained call optimization. + * Only simple method calls that go through the standard indy path can be optimized. + */ + private boolean canUseIndyForChain(final MethodCallExpression call) { + // Spread safe calls need special handling and cannot be optimized + if (call.isSpreadSafe()) return false; + // Super calls have different invocation semantics and should not be optimized + if (isSuperExpression(call.getObjectExpression())) return false; + // This calls and implicit this calls have special context handling + if (isThisExpression(call.getObjectExpression())) return false; + if (call.isImplicitThis()) return false; + // Dynamic method names (non-constant) cannot be handled + String methodName = getMethodName(call.getMethod()); + if (methodName == null) return false; + // "call" method invocations may need special handling for functional interfaces (GROOVY-8466) + if ("call".equals(methodName)) return false; + return true; + } + + /** + * Process a method call expression assuming its receiver is already on the operand stack. + */ + private void makeIndyCallWithReceiverOnStack(final MethodCallExpression call) { + MethodCallerMultiAdapter adapter = invokeMethod; + Expression receiver = call.getObjectExpression(); + if (isSuperExpression(receiver)) { + adapter = invokeMethodOnSuper; + } else if (isThisExpression(receiver)) { + adapter = invokeMethodOnCurrent; + } + + String methodName = getMethodName(call.getMethod()); + if (methodName == null) { + // fallback to normal path which will handle dynamic method names + call.visit(controller.getAcg()); + return; + } + + Expression arguments = call.getArguments(); + boolean safe = call.isSafe(); + boolean containsSpreadExpression = AsmClassGenerator.containsSpreadExpression(arguments); + + OperandStack operandStack = controller.getOperandStack(); + StringBuilder sig = new StringBuilder("(" + getTypeDescription(operandStack.getTopOperand())); + + Label end = null; + if (safe && !isPrimitiveType(operandStack.getTopOperand())) { + operandStack.dup(); + end = operandStack.jump(IFNULL); + } + + // load arguments + int numberOfArguments = 1; + List<Expression> args = makeArgumentList(arguments).getExpressions(); + AsmClassGenerator acg = controller.getAcg(); + if (containsSpreadExpression) { + acg.despreadList(args, true); + sig.append(getTypeDescription(Object[].class)); + } else { + for (Expression arg : args) { + arg.visit(acg); + if (arg instanceof CastExpression) { + operandStack.box(); + acg.loadWrapper(arg); + sig.append(getTypeDescription(Wrapper.class)); + } else { + sig.append(getTypeDescription(operandStack.getTopOperand())); + } + numberOfArguments += 1; + } + } + + sig.append(")Ljava/lang/Object;"); + + String callSiteName = METHOD.getCallSiteName(); + int flags = getMethodCallFlags(adapter, safe, containsSpreadExpression); + + // Note: callSiteName is the invoke-dynamic instruction name, methodName is passed via BSM args + controller.getMethodVisitor().visitInvokeDynamicInsn(callSiteName, sig.toString(), BSM, methodName, flags); + operandStack.replace(OBJECT_TYPE, numberOfArguments); + + if (end != null) controller.getMethodVisitor().visitLabel(end); + } + private void finishIndyCall(final Handle bsmHandle, final String methodName, final String sig, final int numberOfArguments, final Object... bsmArgs) { CompileStack compileStack = controller.getCompileStack(); OperandStack operandStack = controller.getOperandStack(); diff --git a/src/test/groovy/bugs/Groovy7785.groovy b/src/test/groovy/bugs/Groovy7785.groovy index ea9d60d59c..cc69bc11a5 100644 --- a/src/test/groovy/bugs/Groovy7785.groovy +++ b/src/test/groovy/bugs/Groovy7785.groovy @@ -19,17 +19,173 @@ package bugs import org.junit.jupiter.api.Test -import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable import static groovy.test.GroovyAssert.assertScript -@DisabledIfEnvironmentVariable(named = "CI", matches = ".*") // runs locally but fails in CI, more investigation needed +/** + * Tests for GROOVY-7785: StackOverflowError with deeply nested chained method calls. + * Each test uses 500+ chained method calls to verify the fix handles deep chains. + */ final class Groovy7785 { + @Test void testManyChainedMethodCalls() { assertScript ''' - def r = new StringBuilder().append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append(" [...] - assert r.toString() == 'a' * 771 + def r = new StringBuilder().append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append("a").append(" [...] + assert r.toString() == 'a' * 765 + ''' + } + + @Test + void testChainedCaseConversions() { + // 500 chained toUpperCase/toLowerCase calls + assertScript ''' + def result = "hello".toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase().toLowerCase().toUpperCase [...] + assert result == "hello" // 500 is even, so ends with toLowerCase + ''' + } + + @Test + void testChainedReplaceOperations() { + // 500 chained replace calls (none match, string unchanged) + assertScript ''' + def result = "test string".replace("NOTFOUND0", "X").replace("NOTFOUND1", "X").replace("NOTFOUND2", "X").replace("NOTFOUND3", "X").replace("NOTFOUND4", "X").replace("NOTFOUND5", "X").replace("NOTFOUND6", "X").replace("NOTFOUND7", "X").replace("NOTFOUND8", "X").replace("NOTFOUND9", "X").replace("NOTFOUND10", "X").replace("NOTFOUND11", "X").replace("NOTFOUND12", "X").replace("NOTFOUND13", "X").replace("NOTFOUND14", "X").replace("NOTFOUND15", "X").replace("NOTFOUND16", "X").repl [...] + assert result == "test string" + ''' + } + + @Test + void testChainedTrimOperations() { + // 500 chained trim calls (idempotent operation) + assertScript ''' + def result = " hello world ".trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().trim().t [...] + assert result == "hello world" + ''' + } + + @Test + void testChainedCollectionOperations() { + // 500 chained collect calls on a list + assertScript ''' + def result = [1, 2, 3].collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect { it }.collect [...] + assert result == [1, 2, 3] + ''' + } + + @Test + void testChainedBuilderPattern() { + // 500 chained builder set calls + assertScript ''' + class Builder { + private map = [:] + Builder set(String key, value) { + map[key] = value + return this + } + Map build() { map } + } + + def result = new Builder().set("k0", 0).set("k1", 1).set("k2", 2).set("k3", 3).set("k4", 4).set("k5", 5).set("k6", 6).set("k7", 7).set("k8", 8).set("k9", 9).set("k10", 10).set("k11", 11).set("k12", 12).set("k13", 13).set("k14", 14).set("k15", 15).set("k16", 16).set("k17", 17).set("k18", 18).set("k19", 19).set("k20", 20).set("k21", 21).set("k22", 22).set("k23", 23).set("k24", 24).set("k25", 25).set("k26", 26).set("k27", 27).set("k28", 28).set("k29", 29).set("k30", 30).set("k31 [...] + assert result.size() == 500 + assert result.k0 == 0 + assert result.k499 == 499 + ''' + } + + @Test + void testChainedAppendWithDifferentTypes() { + // 500 chained appends with mixed content + assertScript ''' + def sb = new StringBuilder().append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append( [...] + assert sb.length() == 500 + assert sb.toString().startsWith("x" * 200) + assert sb.toString().endsWith("z" * 100) + ''' + } + + @Test + void testChainedSafeNavigationDeep() { + // Deep chain with safe navigation + assertScript ''' + class Node { + Node next + String value = "v" + Node getNext() { next } + } + + // Build a chain of 500 nodes + def root = new Node() + def current = root + 499.times { + current.next = new Node() + current = current.next + } + + // Navigate with chained method calls + def result = root.getNext().getNext().getNext().getNext().getNext() + .getNext().getNext().getNext().getNext().getNext().value + assert result == "v" + ''' + } + + @Test + void testChainedStreamOperations() { + // Deep chain with Java Stream API map operations + assertScript ''' + import java.util.stream.Collectors + + def result = [1, 2, 3].stream().map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it [...] + assert result == [1, 2, 3] + ''' + } + + @Test + void testChainedOptionalOperations() { + // Deep chain with Optional map operations + assertScript ''' + def result = Optional.of(42).map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }.map { it }. [...] + assert result == 42 + ''' + } + + @Test + void testChainedFindAllOperations() { + // 500 chained findAll with always-true predicate + assertScript ''' + def list = [1, 2, 3, 4, 5].findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { true }.findAll { t [...] + assert list == [1, 2, 3, 4, 5] + ''' + } + + @Test + void testMixedChainedOperations() { + // Mix of different operations in one long chain + assertScript ''' + def result = "HELLO".trim().toString().replace("NOTFOUND2", "X").intern().trim().toString().replace("NOTFOUND6", "X").intern().trim().toString().replace("NOTFOUND10", "X").intern().trim().toString().replace("NOTFOUND14", "X").intern().trim().toString().replace("NOTFOUND18", "X").intern().trim().toString().replace("NOTFOUND22", "X").intern().trim().toString().replace("NOTFOUND26", "X").intern().trim().toString().replace("NOTFOUND30", "X").intern().trim().toString().replace("NO [...] + assert result.toLowerCase() == "hello" + ''' + } + + @Test + void testChainedWithClosureReturningThis() { + // Chain with increment operations + assertScript ''' + class Counter { + int count = 0 + Counter increment() { count++; this } + } + + def c = new Counter().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().increment().in [...] + assert c.count == 500 + ''' + } + + @Test + void testVeryDeepChain() { + // Even deeper chain: 1000 calls + assertScript ''' + def sb = new StringBuilder().append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append("x").append( [...] + assert sb.length() == 1000 ''' } }
