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 99462efec4820e058216230da1cb640c6c458c21 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 | 94 +++++++++++- src/test/groovy/bugs/Groovy7785.groovy | 167 ++++++++++++++++++++- 2 files changed, 251 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..7625da0832 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,91 @@ 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 chain of simple method calls that can use indy optimization + List<MethodCallExpression> chain = new ArrayList<>(); + Expression current = receiver; + while (current instanceof MethodCallExpression mce + && !mce.isSpreadSafe() && !mce.isImplicitThis() + && !isSuperExpression(mce.getObjectExpression()) + && !isThisExpression(mce.getObjectExpression())) { + String name = getMethodName(mce.getMethod()); + if (name == null || "call".equals(name)) break; // dynamic name or functional interface call + chain.add(mce); + current = mce.getObjectExpression(); + } + + if (chain.isEmpty()) { + receiver.visit(controller.getAcg()); + return; + } + + // Visit innermost receiver, then process chain from innermost to outermost + current.visit(controller.getAcg()); + AsmClassGenerator acg = controller.getAcg(); + for (int i = chain.size() - 1; i >= 0; i--) { + MethodCallExpression mce = chain.get(i); + acg.onLineNumber(mce, "visitMethodCallExpression: \"" + mce.getMethod() + "\":"); + finishIndyCallForChain(mce); + controller.getAssertionWriter().record(mce.getMethod()); + } + } + + /** Complete an indy call for a chained method, assuming receiver is already on stack. */ + private void finishIndyCallForChain(final MethodCallExpression call) { + OperandStack operandStack = controller.getOperandStack(); + AsmClassGenerator acg = controller.getAcg(); + Expression arguments = call.getArguments(); + boolean safe = call.isSafe(); + + StringBuilder sig = new StringBuilder("(" + getTypeDescription(operandStack.getTopOperand())); + Label end = null; + if (safe && !isPrimitiveType(operandStack.getTopOperand())) { + operandStack.dup(); + end = operandStack.jump(IFNULL); + } + + int nArgs = 1; + List<Expression> args = makeArgumentList(arguments).getExpressions(); + boolean spread = AsmClassGenerator.containsSpreadExpression(arguments); + if (spread) { + 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())); + } + nArgs++; + } + } + sig.append(")Ljava/lang/Object;"); + + int flags = safe ? SAFE_NAVIGATION : 0; + if (spread) flags |= SPREAD_CALL; + controller.getMethodVisitor().visitInvokeDynamicInsn(METHOD.getCallSiteName(), sig.toString(), BSM, getMethodName(call.getMethod()), flags); + operandStack.replace(OBJECT_TYPE, nArgs); + 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..375a9b3209 100644 --- a/src/test/groovy/bugs/Groovy7785.groovy +++ b/src/test/groovy/bugs/Groovy7785.groovy @@ -19,17 +19,176 @@ 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 1000 chained method calls to verify the fix handles deep chains. + */ final class Groovy7785 { + @Test void testManyChainedMethodCalls() { + // 1000 chained append calls on StringBuilder + 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' * 1000 + ''' + } + + @Test + void testChainedCaseConversions() { + // 1000 chained toUpperCase/toLowerCase calls 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 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" // 1000 is even, ends with toLowerCase ''' } + + @Test + void testChainedReplaceOperations() { + // 1000 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() { + // 1000 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() { + // 1000 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() { + // 1000 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() == 1000 + assert result.k0 == 0 + assert result.k999 == 999 + ''' + } + + @Test + void testChainedAppendWithDifferentTypes() { + // 1000 chained appends with mixed content (400x + 400y + 200z) + 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 + assert sb.toString().startsWith("x" * 400) + assert sb.toString().endsWith("z" * 200) + ''' + } + + @Test + void testChainedNavigationDeep() { + // 1000 chained getNext() calls + assertScript ''' + class Node { + Node next + String value = "v" + Node getNext() { next } + } + + // Build a chain of 1001 nodes + def root = new Node() + def current = root + 1000.times { + current.next = new Node() + current = current.next + } + + // Navigate through 1000 nodes + def nav = root.getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNext().getNe [...] + assert nav.value == "v" + assert nav.next == null + ''' + } + + @Test + void testChainedStreamOperations() { + // 1000 chained stream 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() { + // 1000 chained 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() { + // 1000 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() { + // 1000 mixed string operations (trim/toString/replace/intern) + 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() { + // 1000 chained 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 == 1000 + ''' + } + + @Test + void testChainedSafeCallOperations() { + // 1000 chained safe navigation calls (?.) + assertScript ''' + def str = " hello world " + def result = str?.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() [...] + assert result == "hello world" + ''' + } + }
