You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "paulk-asert (via GitHub)" <gi...@apache.org> on 2026/08/18 10:21:26 UTC
[PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
paulk-asert opened a new pull request, #2813:
URL: https://github.com/apache/groovy/pull/2813
…r calls
Closure.checkForReferenceCycle walks owner, delegate and thisObject, which are the fields a closure dispatches through. A closure which wraps another and invokes it recurses through its own field instead, so a forged graph cycling there passed the check and then exhausted the stack on invocation.
Measured before the change, building each cycle by reflection, serializing and reading it back:
CurriedClosure, cycle via owner rejected, InvalidObjectException
ComposedClosure, cycle via first/second accepted, then StackOverflowError
TrampolineClosure, cycle via original accepted, then StackOverflowError
ComposedClosure was the more surprising of the two: it already opts into the check from its readResolve, so the check ran and had nothing to say about the closure's own recursion. TrampolineClosure had no readResolve at all.
Add Closure.additionalReferences for a subclass to declare the closures it calls through, and walk those as well. ComposedClosure declares first and second; TrampolineClosure declares original and gains the readResolve it was missing.
Only fields a closure calls belong there, which is why the walk is not simply made reflective over every Closure-valued field: a captured closure is not a recursion edge, and treating it as one would reject graphs that invoke perfectly well.
The existing ComposedClosure cycle test forges its cycle through owner and delegate, so it exercised the walk that already worked; the new tests forge through the wrapped fields, and fail without this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2813:
URL: https://github.com/apache/groovy/pull/2813#issuecomment-5328201122
## ✅ All tests passed ✅
🏷️ Commit: 99aff08f822c47280ae70500a5587d8027025fa3
▶️ Tests: 110480 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert merged PR #2813:
URL: https://github.com/apache/groovy/pull/2813
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2813:
URL: https://github.com/apache/groovy/pull/2813#issuecomment-5336345699
## ✅ All tests passed ✅
🏷️ Commit: cb2864de0b107490fd2ed3fd6836e80dc53fe000
▶️ Tests: 110155 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2813:
URL: https://github.com/apache/groovy/pull/2813#discussion_r3809048711
##########
src/main/java/groovy/lang/Closure.java:
##########
@@ -1480,6 +1482,33 @@ protected static void checkForReferenceCycle(final Closure<?> root) throws Inval
}
}
+ private static Object[] concat(final Object[] first, final Object[] second) {
+ if (second.length == 0) return first;
+ final Object[] joined = new Object[first.length + second.length];
Review Comment:
`concat` assumes `second` is non-null (`second.length`). While `additionalReferences()` documents that it never returns null, this is a protected API and an accidental null override would turn deserialization validation into a confusing `NullPointerException`. Consider treating `null` as “no additional references” for robustness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12276: Extend the closure cycle check to the closures a wrappe… [groovy]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2813:
URL: https://github.com/apache/groovy/pull/2813#issuecomment-5327553278
## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2813?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 70.1856%. Comparing base ([`f140841`](https://app.codecov.io/gh/apache/groovy/commit/f140841cf98d79f01e9fde689a46e696ca4493ae?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`99aff08`](https://app.codecov.io/gh/apache/groovy/commit/99aff08f822c47280ae70500a5587d8027025fa3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2813?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2813 +/- ##
==================================================
+ Coverage 70.1837% 70.1856% +0.0019%
- Complexity 35844 35853 +9
==================================================
Files 1562 1562
Lines 132528 132540 +12
Branches 24379 24379
==================================================
+ Hits 93013 93024 +11
- Misses 31106 31109 +3
+ Partials 8409 8407 -2
```
| [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2813?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [src/main/java/groovy/lang/Closure.java](https://app.codecov.io/gh/apache/groovy/pull/2813?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Flang%2FClosure.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvbGFuZy9DbG9zdXJlLmphdmE=) | `85.0550% <100.0000%> (+0.2675%)` | :arrow_up: |
| [src/main/java/groovy/lang/TrampolineClosure.java](https://app.codecov.io/gh/apache/groovy/pull/2813?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Flang%2FTrampolineClosure.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvbGFuZy9UcmFtcG9saW5lQ2xvc3VyZS5qYXZh) | `76.4706% <100.0000%> (+5.0420%)` | :arrow_up: |
| [...a/org/codehaus/groovy/runtime/ComposedClosure.java](https://app.codecov.io/gh/apache/groovy/pull/2813?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FComposedClosure.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvQ29tcG9zZWRDbG9zdXJlLmphdmE=) | `77.2727% <100.0000%> (+1.0823%)` | :arrow_up: |
... and [8 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2813/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org