Baymine opened a new pull request, #66038:
URL: https://github.com/apache/doris/pull/66038
### What problem does this PR solve?
Issue Number: close #66037
Problem Summary:
JNI scan threads are pooled and reused across queries. A JNI exception is
sticky per-thread: it stays pending until explicitly cleared. If one JNI call
leaves an exception pending and it is not cleared, the next JNI call on that
same reused thread runs under a pending exception (undefined behavior) and the
error gets misattributed to an unrelated call, which can fail an otherwise
healthy query. It also skips off-heap table release, leaking memory.
There are two sides to the bug:
- FE (`JniScanner.getNextBatchMeta`): the batch loop only caught
`IOException` from `getNext()`. Scanner subclasses (and the libraries they
call) routinely surface failures as unchecked `RuntimeException`/NPE. When such
an exception escaped, `releaseTable()` was skipped (off-heap `VectorTable`
leak) and a bare pending JNI exception was left on the pooled scan thread,
later misattributed to an unrelated JNI call such as `getStatistics` and able
to fail the whole query. The fix catches `Throwable`, always calls
`releaseTable()`, and normalizes non-`IOException` throwables to a checked
`IOException` (keeping the original as cause) so BE clears it cleanly.
- BE (`JniReader::_get_statistics` / `JniReader::close` and
`JniTableReader::_get_statistics`): these best-effort methods ran JNI calls
while a stale pending exception (left by an earlier call on the same pooled
thread) could still be present. In `close()` the early `RETURN_ERROR_IF_EXC`
aborted before the resource-release/close calls, leaking the Java scanner and
off-heap table. The real scan error is already carried by the scan status, so
each method now discards any residual pending exception up front (logging it)
before touching the JVM again, so profile collection and release/close always
run on a clean thread state.
### Release note
Fix off-heap memory leak and spurious query failures when a JNI scanner's
getNext throws an unchecked exception.
### Check List (For Author)
- Test
- [x] Unit Test: added
`JniScannerTest.testGetNextBatchMetaNormalizesUncheckedException` and
`testGetNextBatchMetaPreservesIOException` (a minimal `JniScanner` whose
`getNext()` throws is used to assert `getNextBatchMeta()` always releases the
table and normalizes the throwable to `IOException`). BE: both changed
translation units compile clean and `clang-format` is clean. No BE unit test
for the stale-exception guard because reproducing it needs a live JVM and a
reused pooled scan thread.
- Behavior changed:
- [x] No.
- Does this need documentation?
- [x] No.
--
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]
