sandeshkr419 commented on code in PR #1240:
URL: https://github.com/apache/arrow-java/pull/1240#discussion_r3624630692
##########
c/src/main/java/org/apache/arrow/c/ReferenceCountedArrowArray.java:
##########
@@ -64,13 +64,18 @@ void release() {
*/
ArrowBuf unsafeAssociateAllocation(
BufferAllocator trackingAllocator, long capacity, long memoryAddress) {
+ // Retain AFTER wrap: wrapForeignAllocation throws OutOfMemoryException
when the allocator is
+ // over its limit, and a retain() before that throw would leave the count
elevated with no
+ // matching release0(), preventing the array's release callback from ever
firing.
+ ArrowBuf buf =
+ trackingAllocator.wrapForeignAllocation(
Review Comment:
The retain-after pattern is intentionally correct for both throw paths in
`BaseAllocator.wrapForeignAllocation`:
- Path 1 (allocator-limit OOM, lines 261–269): throws before the try block,
so release0() is never called. Retaining first would leave the count elevated
with nothing to balance it.
- Path 2 (unexpected error inside the try, lines 279–290): BaseAllocator's
own catch already calls `release0() `before rethrowing. Retaining first and
catching here to balance would cause a double-release on that path.
The retain-before + explicit-catch approach you suggest would fix path 1 but
double-release on path 2. Retaining only on success handles both correctly.
--
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]
