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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
