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

ARROW-10499: [C++][Java] Fix ORC Java JNI Crash #8595

Closed
wants to merge 1 commit into from

Conversation

terencehonles
Copy link
Contributor

@terencehonles terencehonles commented Nov 5, 2020

OrcStripeReaderJniWrapper::getSchema previously used nullptr for the memory pool parameter to arrow::ipc::SerializeSchema to implicitly call arrow::default_memory_pool().

As part of ARROW-10080 (#8533), a check was placed to fail on nullptr being provided. This change removes the check, but also explicitly calls arrow::default_memory_pool() which is used elsewhere in the JNI wrapper.

`OrcStripeReaderJniWrapper::getSchema` previously used `nullptr` for the
memory pool parameter to `arrow::ipc::SerializeSchema` to implicitly
call `arrow::default_memory_pool()`.

As part of https://issues.apache.org/jira/browse/ARROW-10080, a check
was placed to fail on `nullptr` being provided. This change removes the
check, but also explicitly calls `arrow::default_memory_pool()` which is
used elsewhere in the JNI wrapper.
@github-actions
Copy link

github-actions bot commented Nov 5, 2020

@kiszk
Copy link
Member

kiszk commented Nov 5, 2020

LGTM

@terencehonles
Copy link
Contributor Author

@kiszk you mind merging this in? Not sure if anything else needs to happen, but I don't have write access to the repo 🙂

@kiszk
Copy link
Member

kiszk commented Nov 5, 2020

I do not have the grant to merge it, too. I expect that others will look at this.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

I'll merge this.

@bkietz You can try re-adding the DCHECK_NE again later if you want.

@kou kou closed this in cb4f26d Nov 5, 2020
@terencehonles terencehonles deleted the arrow-10499 branch November 5, 2020 21:36
@BryanCutler
Copy link
Member

Thanks for fixing this @terencehonles !

GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
`OrcStripeReaderJniWrapper::getSchema` previously used `nullptr` for the memory pool parameter to `arrow::ipc::SerializeSchema` to implicitly call `arrow::default_memory_pool()`.

As part of ARROW-10080 (apache#8533), a check was placed to fail on `nullptr` being provided. This change removes the check, but also explicitly calls `arrow::default_memory_pool()` which is used elsewhere in the JNI wrapper.

Closes apache#8595 from terencehonles/arrow-10499

Authored-by: Terence D. Honles <terence@honles.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants