[CI only] Attempt a fix to a potential problem in RecursionOverflow #5862
Conversation
This reverts commit bab1540.
abgruszecki
force-pushed
the
ci/recursion-of-recursions
branch
from
c23d5fc to
c0a4ed0
Compare
There was a problem hiding this comment.
Whether or not this is the bug, the change to recursions look very reasonable: the current definition is not tail-recursive, so it has no business being invoked in stack-overflow-related code. Notice the code in handleRecursive creating the RecursionOverflow chain uses a while loop for the same reason.
Not sure if we can re-enable the flaky test, more fixes might be needed.
| val result = ListBuffer.empty[RecursionOverflow] | ||
| @annotation.tailrec def loop(throwable: Throwable): List[RecursionOverflow] = throwable match { | ||
| case ro: RecursionOverflow => | ||
| if (result.contains(ro)) println("caught the issue") |
There was a problem hiding this comment.
It’d be very surprising if the RecursionOverflow chain was cyclic. However, the length of the chain is proportional to the stack depth, so it’s best traversed in constant stack size. To see why, you need to see how RecursionOverflow is created by handleRecursive and how that’s used in certain recursive methods in Typer (I can explain in person if desired).
There was a problem hiding this comment.
Yeah, I wasn't sure if the RecursionOverflow chain can actually be cyclic. If it's really proportional to stack depth, then it makes sense to construct recursions in a loop. How about I simply open a separate PR with that change and we merge it on its own?
|
Closing, as I could not replicate the mysterious |
This is an attempt at a fix to the potential problem that causes #5860. The stack overflow looks as follows:
I cannot replicate the problem locally in a simple way, even if I add a test like this:
Seems triggering it depends in some way on current VM state. I'll try to run the CI a few times and see if I can catch the issue.