GROOVY-11022: StackOverflowError when having parameterized function w…#2496
GROOVY-11022: StackOverflowError when having parameterized function w…#2496paulk-asert wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2496 +/- ##
==================================================
+ Coverage 67.1374% 67.1396% +0.0021%
- Complexity 31622 31627 +5
==================================================
Files 1451 1451
Lines 122565 122573 +8
Branches 22007 22009 +2
==================================================
+ Hits 82287 82295 +8
+ Misses 33199 33197 -2
- Partials 7079 7081 +2
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes GROOVY-11022 (StackOverflowError during static type checking) by adding a recursion guard when expanding generic placeholder bounds, and adds a regression test covering F-bounded type parameters with self-references inside wildcard bounds.
Changes:
- Add a per-thread “currently expanding bounds” stack to prevent infinite recursion in
applyGenericsContextfor F-bounded generics with wildcard self-references. - Add STC regression tests for
? super/? extendsself-referential bounds patterns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/test/groovy/groovy/transform/stc/GenericsSTCTest.groovy | Adds regression coverage for the StackOverflowError scenario involving recursive bounds inside wildcards. |
| src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java | Introduces a recursion guard during generics bound expansion to avoid infinite recursion. |
…ith recursive bounds
b841622 to
fad65c6
Compare
| } | ||
|
|
||
| private static final ThreadLocal<Deque<GenericsTypeName>> EXPANDING_BOUNDS = | ||
| ThreadLocal.withInitial(ArrayDeque::new); |
There was a problem hiding this comment.
I am very critical to using a ThreadLocal. At the very least there should be a description of why this has to be ThreadLocal
| newGT.setPlaceholder(true); | ||
| return newGT; | ||
| } finally { | ||
| expanding.pop(); |
There was a problem hiding this comment.
so we push to expanding and then pop from expanding. That means we remove the entry we just added, even in the case of line 1806, with the return. Why is it then not always empty again? Ah, because of the recursive call applyGenericsContext(spec, gt.getLowerBound()). But why does it have to be a ThreadLocal then? I would prefere going for a breaking change here nstead of using ThreadLocal
|
Abandoned, instead the slightly larger solution for master will be back-ported. |
…ith recursive bounds
This is the fix recommended for GROOVY_5_0_X