dweiss opened a new pull request, #16549:
URL: https://github.com/apache/lucene/pull/16549
The `testRecursionDepth*` tests added in #16423 assert exact error offsets
from the nesting-depth listener and we've seen a number of CI errors that
triggered from these methods - typically on coverage and s390x machines. This
wasn't reproducing for me, but I let Claude rip through the potential causes
and it found one -
```
Root cause: two error paths race. `JavascriptNestingDepthListener` needs
~`MAX_NESTING_DEPTH`
(1025) levels of real ANTLR parser recursion before it can throw with the
exact offset; the
`catch (StackOverflowError)` fallback in
`JavascriptCompiler.compileExpression()` reports
offset `-1`. Which one wins depends on how many bytes each `expression()`
frame costs — a
function of JIT tier, not anything the reproduce line captures. Frame
sizes shift with
compilation state: in an experiment compiling `"-".repeat(20000) + "1"`
repeatedly on a 600 KB
stack in one JVM, iteration 0 (interpreted) produced offset 1024 while
iterations 1–2 (after
C1/C2 kicked in) hit the SOE fallback and produced -1. On a default 1 MB
stack the worst-case
consumption to reach depth 1025 measured ~600 KB, so CI environments with
fatter frames
(coverage instrumentation, randomized JVM flags, different JDK builds) can
cross the line.
Deterministic repro of the CI failure before this fix:
gradlew -p lucene/expressions test --tests
"TestJavascriptCompiler.testRecursionDepth2" -Dtests.jvmargs="-Xss300k"
```
Indeed, it does reproduce the problem and is a sounds explanation of what's
happening. I also like the suggested fix (to the test):
```
Fix (test-only; the SOE fallback in production code is intentional and
unchanged):
`assertRecursionLimit` now compiles on a dedicated thread with an explicit
16 MB stack and
rethrows whatever the compiler threw, so the depth listener
deterministically wins and the
exact-offset assertions (`1024` for depth-limited input, `0` for the
depth-first case in
`testRecursionDepth3`) remain meaningful. All five `testRecursionDepth*`
tests funnel through
the helper.
```
I don't think this can/should be fixed in the source code - it just ensures
the test really hits the limit (which is high).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
