You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tustvold (via GitHub)" <gi...@apache.org> on 2023/03/21 13:08:31 UTC
[GitHub] [arrow-rs] tustvold opened a new pull request, #3893: Fix pyarrow memory leak (#3683)
tustvold opened a new pull request, #3893:
URL: https://github.com/apache/arrow-rs/pull/3893
Remove ArrowArray::into_raw and try_from_raw
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #3683
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
This is a follow on to https://github.com/apache/arrow-rs/pull/3685 that removes the deprecated methods, and fixes a remaining memory leak.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
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
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3893: Fix pyarrow memory leak (#3683)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3893:
URL: https://github.com/apache/arrow-rs/pull/3893#discussion_r1143367177
##########
arrow-data/src/ffi.rs:
##########
@@ -118,9 +118,6 @@ struct ArrayPrivateData {
impl FFI_ArrowArray {
/// creates a new `FFI_ArrowArray` from existing data.
- /// # Memory Leaks
Review Comment:
This comment is out-of-date, `FFI_ArrowArray` frees memory automatically on drop
--
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
[GitHub] [arrow-rs] viirya commented on a diff in pull request #3893: Fix pyarrow memory leak (#3683)
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #3893:
URL: https://github.com/apache/arrow-rs/pull/3893#discussion_r1143659643
##########
arrow-data/src/ffi.rs:
##########
@@ -118,9 +118,6 @@ struct ArrayPrivateData {
impl FFI_ArrowArray {
/// creates a new `FFI_ArrowArray` from existing data.
- /// # Memory Leaks
Review Comment:
I guess that it means that consumer of `FFI_ArrowArray` (might be in other language like JVM) should call `release` as C Data Interface defines to release 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3893: Fix pyarrow memory leak (#3683)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3893:
URL: https://github.com/apache/arrow-rs/pull/3893#discussion_r1143693686
##########
arrow-data/src/ffi.rs:
##########
@@ -118,9 +118,6 @@ struct ArrayPrivateData {
impl FFI_ArrowArray {
/// creates a new `FFI_ArrowArray` from existing data.
- /// # Memory Leaks
Review Comment:
Perhaps, but from the context of Rust it is just a little bit confusing, as `release` isn't even publicly visible :sweat_smile:
--
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
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3893: Fix pyarrow memory leak (#3683)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3893:
URL: https://github.com/apache/arrow-rs/pull/3893#discussion_r1143426020
##########
arrow/src/array/ffi.rs:
##########
@@ -66,6 +51,7 @@ pub unsafe fn make_array_from_raw(
/// This function copies the content of two FFI structs [ffi::FFI_ArrowArray] and
/// [ffi::FFI_ArrowSchema] in the array to the location pointed by the raw pointers.
/// Usually the raw pointers are provided by the array data consumer.
+#[deprecated(note = "Use FFI_ArrowArray::new and FFI_ArrowSchema::try_from")]
Review Comment:
The above methods are not only safe, but less likely to accidentally leak memory
--
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
[GitHub] [arrow-rs] tustvold merged pull request #3893: Fix pyarrow memory leak (#3683)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #3893:
URL: https://github.com/apache/arrow-rs/pull/3893
--
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