pingzh commented on code in PR #5722:
URL: https://github.com/apache/datafusion-comet/pull/5722#discussion_r3945022557
##########
spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/NativeBatchDecoderIterator.scala:
##########
@@ -225,6 +233,7 @@ case class NativeBatchDecoderIterator(
if (previous != null) release(previous.close())
prefetched.filterNot(_ eq previous).foreach(pending =>
release(pending.close()))
+ if (decoderHandle != 0L)
release(nativeLib.releaseRemoteShuffleDecoder(decoderHandle))
Review Comment:
Codex acting for the user: Fixed the `NativeBatchDecoderIterator` lifetime
race in
[92fa68c5b](https://github.com/apache/datafusion-comet/commit/92fa68c5b62894e758406d0d0ae91ef4a16f4d75).
Decoder creation, native decoding, Arrow import, and batch publication now
share the monitor used by `close()`. `next()` also transfers batch ownership
under that monitor. A transport read that finishes after cleanup rechecks
`isClosed` before creating or using a decoder.
`readNextBlock()` stays outside the monitor so `in.close()` can unblock it.
Native decode failures unwind Arrow cleanup before reporting outside the
monitor, since reporting may perform an RPC.
Added five latch-based regressions covering creation, active decoding
(including interrupted cleanup), import/publication, blocked transport reads,
and failure reporting. All five failed before the fix; all 69 reader-suite
tests now pass on both Spark 4.1 and Spark 3.4.3. Spotless and Scalastyle also
pass.
The broader `CometExecIterator` lifetime concern remains a separate
follow-up.
--
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]
