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.

Sign up for GitHub

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

Jump to bottom

ARROW-15234: [Python] Fix crash with custom CSV invalid row handler #12063

Closed
wants to merge 1 commit into from
Closed

ARROW-15234: [Python] Fix crash with custom CSV invalid row handler #12063

wants to merge 1 commit into from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Jan 3, 2022

The crash would happen at handler destruction if called from a non-Python thread that doesn't hold the GIL.

@pitrou
ARROW-15234: [Python] Fix crash with custom CSV invalid row handler
c4aeeb5 
The crash would happen at handler destruction if called from a non-Python thread that doesn't hold the GIL.
@github-actions
Copy link

github-actions bot commented Jan 3, 2022

https://issues.apache.org/jira/browse/ARROW-15234

@github-actions
Copy link

github-actions bot commented Jan 3, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@pitrou pitrou requested a review from lidavidm January 3, 2022 16:03
lidavidm
lidavidm approved these changes Jan 3, 2022
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

I suppose the test from the previous PR only fails nondeterministically, is it worth trying to test this more specifically?

@lidavidm
Copy link
Member

lidavidm commented Jan 3, 2022

Ah, I do see it failing elsewhere in CI. I think the existing tests are sufficient then and we should get in the fix.

@lidavidm lidavidm closed this in 47f6bc3 Jan 3, 2022
@ursabot
Copy link

ursabot commented Jan 3, 2022 •
edited
Loading

Benchmark runs are scheduled for baseline = c6143a2 and contender = 47f6bc3. 47f6bc3 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
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Failed] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@pitrou pitrou deleted the ARROW-15234-invalid-row-handler-crash branch January 3, 2022 17:20
@asfimport asfimport mentioned this pull request Jan 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidavidm lidavidm lidavidm approved these changes

Assignees
No one assigned
Labels
Component: C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pitrou @lidavidm @ursabot