Skip to content

ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter#12816

Closed
raulcd wants to merge 5 commits into
apache:masterfrom
raulcd:ARROW-16025
Closed

ARROW-16025: [Python][C++] Fix segmentation fault when closing ORCFileWritter#12816
raulcd wants to merge 5 commits into
apache:masterfrom
raulcd:ARROW-16025

Conversation

@raulcd

@raulcd raulcd commented Apr 6, 2022

Copy link
Copy Markdown
Member

This PR fixes ARROW-16025 (Calling nonexistent method of pyarrow.orc.ORCWriter causes segfault).

The segmentation fault can be reproduced with the test:

def test_wrong_usage_orc_writer(tempdir):
    from pyarrow import orc

    path = str(tempdir / 'test.orc')
    with orc.ORCWriter(path) as writer:
        with pytest.raises(AttributeError):
            writer.test()

The issue was that closing ORCFileWriter without actually writing was trying to close a null writer (writer_->close();) causing the segmentation fault.

…g to close ORCFileWriter without having a writer
@github-actions

github-actions Bot commented Apr 6, 2022

Copy link
Copy Markdown

@raulcd raulcd marked this pull request as ready for review April 7, 2022 08:52
@raulcd

raulcd commented Apr 7, 2022

Copy link
Copy Markdown
Member Author

As discussed with @jorisvandenbossche I have added a test that was causing a segmentation fault previously as seen on ARROW-15723 which is solved as part of this fix.

@jorisvandenbossche jorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks a lot!

Comment thread cpp/src/arrow/adapters/orc/adapter_test.cc Outdated
@amol-

amol- commented Apr 13, 2022

Copy link
Copy Markdown
Member

The failed test is unrelated, merging

@amol- amol- closed this in ccaef09 Apr 13, 2022
@ursabot

ursabot commented Apr 14, 2022

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 5441c4b and contender = ccaef09. ccaef09 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️1.17% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.17% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/498| ccaef092 ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/484| ccaef092 test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/484| ccaef092 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/494| ccaef092 ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/497| 5441c4b1 ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/483| 5441c4b1 test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/483| 5441c4b1 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/493| 5441c4b1 ursa-thinkcentre-m75q>
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants