[GitHub] [arrow] pitrou commented on pull request #35996: GH-35988: [C#] The C data interface implementation can leak on import

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou commented on PR #35996:
URL: https://github.com/apache/arrow/pull/35996#issuecomment-1582941222

   Hmm, so there are two (unrelated?) changes here.
   
   The first change (removing the `cArray->release != null` checks in export functions) looks fine.
   
   The second change is IMHO the symptom of an implementation issue. `ImportedArrowArray` currently stores the given raw pointer `CArrowArray* _cArray`. Instead, it should probably store its own `CArrowArray` struct like the C++ importer does:
   https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/bridge.cc#L1241-L1261
   
   Then, importing an array _moves_ the user-supplied structure to the owned structure, like this:
   https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/bridge.cc#L1285-L1287
   
   Therefore, the imported array structure does not depend on the lifetime of the user-supplied pointer.
   
   By the way, moving is cheap so you shouldn't fear any inefficiency here:
   https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/helpers.h#L66-L75
   


-- 
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