You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "pepijnve (via GitHub)" <gi...@apache.org> on 2025/05/22 09:29:31 UTC

[I] Data import methods free memory [arrow-java]

pepijnve opened a new issue, #763:
URL: https://github.com/apache/arrow-java/issues/763

   ### Describe the usage question you have. Please include as many useful details as possible.
   
   
   The various import methods in the Data class are documented to transfer ownership of the Arrow data from the input object to internal values as per the C FFI. If I'm reading https://arrow.apache.org/docs/format/CDataInterface.html#moving-an-array correctly, implementations are supposed to hollow out the input object in that case by nulling the release field. In practice a class like ArrayImporter does this
   
   ```
   void importArray(ArrowArray src) {
     ...
     // Move imported array
     ArrowArray ownedArray = ArrowArray.allocateNew(allocator);
     ownedArray.save(snapshot);
     src.markReleased();
     src.close();
     ...
   }
   ```
   
   So not only is the array marked as released, it is also closed so the ArrowArray instance is no longer usable. This may or may not happen when you call `Data#importIntoVector` for instance, since that might abort early when given a null `Allocator` which makes the post conditions of `Data#importIntoVector` a bit unclear. You kind of expect the ArrowArray to have been closed, but you still need to call close yourself as well or risk having a memory leak.
   
   What's the rationale behind the choice to also call `close` on objects passed to Data rather than expecting people to use try-with-resources/try-finally themselves?


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Data import methods free memory [arrow-java]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #763:
URL: https://github.com/apache/arrow-java/issues/763#issuecomment-2903439789

   New methods is probably cleanest


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Data import methods free memory [arrow-java]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #763:
URL: https://github.com/apache/arrow-java/issues/763#issuecomment-2903047419

   Basically, the idea is that the importer should not free the `struct ArrowArray` itself, so that the caller code can use its own try-with-resources? I think that makes sense. (Well, either that, or the importer internally should free the struct even if it errors, which I think is less clean but would be less of a breaking change.)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Data import methods free memory [arrow-java]

Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve closed issue #763: Data import methods free memory
URL: https://github.com/apache/arrow-java/issues/763


-- 
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: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Data import methods free memory [arrow-java]

Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on issue #763:
URL: https://github.com/apache/arrow-java/issues/763#issuecomment-2903411659

   > Basically, the idea is that the importer should not free the struct ArrowArray itself, so that the caller code can use its own try-with-resources?
   
   I think that would make more sense personally, but it would be a serious backwards incompatible change. The alternative would be to add additional methods (perhaps an overloaded method signature with a boolean parameter, although that's not really pretty either) that let the caller control what happens.
   
   I would be happy to put together a PR for this if you like. Any preference on what this should look like exactly?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Data import methods free memory [arrow-java]

Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on issue #763:
URL: https://github.com/apache/arrow-java/issues/763#issuecomment-2903452618

   I took a first stab at this in #766. The PR crossed your comment, so it's the initial version is the breaking change approach. I'll close this discussion issue and continue in the PR.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org