wangfenjin commented on issue #1425:
URL: https://github.com/apache/arrow-rs/issues/1425#issuecomment-1065799969
I get your point. You are calling rust from Java, so it's right you alloc
the memory in Java. The problem is rust assume this pointer is from
Arc::into_raw, but acctually it's not it's just a raw pointer from JNI. In this
case, our solution should be add a new API like:
```rust
/// creates a new [ArrowArray] from two pointers. Used to import from
the C Data Interface.
/// # Safety
/// See safety of [ArrowArray]
/// # Error
/// Errors if any of the pointers is null
pub unsafe fn try_from_c(
array: *const FFI_ArrowArray,
schema: *const FFI_ArrowSchema,
) -> Result<Self> {
if array.is_null() || schema.is_null() {
return Err(ArrowError::MemoryError(
"At least one of the pointers passed to `try_from_raw` is
null"
.to_string(),
));
};
Ok(Self {
array: Arc::new(*array), // notice we don't need to clone here,
if will also solve your problem
schema: Arc::new(*schema),
})
}
```
The idea is we don't need to Clone, why bother? As in your Java program you
need to hold the pointer as long as you need id, and can only free when the job
is done. Again, "the one who allocate it should free it".
There are several drawbacks in the current solution:
1. It breaks current API convention. Anyone use the API like me will be
detected memory leak because of this change.
2. You can say your Java program don't leak memory because you always
holding the pointer, but in some aspect we can say it's, because we alloc two
struct point to the same data, which can be easily and should managed in one.
3. Allow clone will make us have the same issue mentioned by @jorgecarleitao
, I add a demo here https://github.com/apache/arrow-rs/pull/1431
About the link you mentioned [Moving an
array](https://arrow.apache.org/docs/format/CDataInterface.html#moving-an-array),
please noted that it talks about *consumer* which is rust in this case, need
to call release_call if it tries to move the data, seems have nothing to do
with our discuess here.
> If you really want to free it, make sure to clear up the release field in
the struct. This is easy to do with Java FFI API.
In here you are talking about the producer side, but actually you can't just
clear up the release field and forget about it. What if rust have any issue to
release the data, Java as the producer, as the one alloc the memory, should
still be responsible to release it. Again, "the one who allocate it should free
it".
--
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]
