Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Go] Exported ArrowArrayStream.get_next doesn't handle uninitialized ArrowArrays well #33767

Closed
lidavidm opened this issue Jan 18, 2023 · 3 comments · Fixed by #33768
Closed
Assignees
Milestone

Comments

@lidavidm
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

get_next should set ArrowArray.release to NULL when there are no more records. However, the current implementation instead tries to release the out-parameter. This is harmless when the out-parameter is 0-initialized (the implementation will skip the call) but otherwise it'll crash (after jumping to a random garbage address).

Component(s)

Go

@lidavidm
Copy link
Member Author

@pitrou I noticed this while using PyArrow to import an ArrowArrayStream exported from Go. PyArrow apparently passes in an uninitialized ArrowArray to Go, and Go was assuming it would be initialized. Should we say that producers should not assume inputs are initialized, or that consumers should (zero-)initialize parameters?

@pitrou
Copy link
Member

pitrou commented Jan 19, 2023

The out parameter should be handled as an out-parameter, so it can't be expected to be initialized (to what?).

@lidavidm
Copy link
Member Author

That's what I thought :)

(It was expecting it to be zero-initialized, because it would call the release callback if present.)

lidavidm added a commit that referenced this issue Jan 19, 2023
* Closes: #33767

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 12.0.0 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants