[GitHub] [arrow-rs] wangfenjin commented on a change in pull request #1449: Remove Clone and copy source structs internally

[GitBox <gi...@apache.org>]

wangfenjin commented on a change in pull request #1449:
URL: https://github.com/apache/arrow-rs/pull/1449#discussion_r827638323



##########
File path: arrow/src/ffi.rs
##########
@@ -781,11 +781,19 @@ impl ArrowArray {
                     .to_string(),
             ));
         };
-        let ffi_array = (*array).clone();
-        let ffi_schema = (*schema).clone();
+
+        let array_mut = array as *mut FFI_ArrowArray;
+        let schema_mut = schema as *mut FFI_ArrowSchema;
+
+        let array_data = std::ptr::replace(array_mut, FFI_ArrowArray::empty());

Review comment:
       > For instance, if array and schema are from Arc::into_raw, then the memory allocated for the Arc will become dangling after this, and thus memory leak.
   
   Currently if user try to export using into_raw and and don't import using from_raw (we can assume it's a normal case? as they export data to be used by others they don't need to import again), they might have memory leak.
   
   After check the [CPP-import implementation](https://github.com/apache/arrow/blob/8e43f23dcc6a9e630516228f110c48b64d13cec6/cpp/src/arrow/c/helpers.h#L63-L75), I think this change is fine. We even can remove the two drop_in_place call as it seems unnecessary.
   
   What we need is to redesign the ArrowArray::into_raw(), we can't use Arc::into_raw in the implementation




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