ARROW-404: [Python] Fix segfault caused by HdfsClient getting closed before an HdfsFile #230
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hen the last user-accessible client reference goes out of scope Change-Id: If97731c7cd663d89e418b4cdd713eb8f0add49e7
Change-Id: I4cf14f47203be94a8a95f5c7d9b780734a8cec67
xhochy
reviewed
Dec 8, 2016
| def __dealloc__(self): | ||
| self.file_handle.close() | ||
| # try to avoid cyclic GC | ||
| self.file_handle = None |
There was a problem hiding this comment.
Could we get around the cyclic GC problem by having a weak reference here to the file handle? (Disclaimer: I have only used weak references in C++, not in Python nor Cython, so I have no idea if that would really work.)
Change-Id: I9f2b03e3f9e8677c89f144ff890edc3900431a40
|
Cool that worked |
wesm
pushed a commit
to wesm/arrow
that referenced
this pull request
Sep 2, 2018
Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#230 from majetideepak/PARQUET-846 and squashes the following commits: 9007986 [Deepak Majeti] PARQUET-846: CpuInfo::Init() is not thread safe
wesm
pushed a commit
to wesm/arrow
that referenced
this pull request
Sep 4, 2018
Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#230 from majetideepak/PARQUET-846 and squashes the following commits: 9007986 [Deepak Majeti] PARQUET-846: CpuInfo::Init() is not thread safe Change-Id: I33bef39d6291dfe207bfc248cf7ce571f6308ad3
wesm
pushed a commit
to wesm/arrow
that referenced
this pull request
Sep 6, 2018
Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#230 from majetideepak/PARQUET-846 and squashes the following commits: 9007986 [Deepak Majeti] PARQUET-846: CpuInfo::Init() is not thread safe Change-Id: I33bef39d6291dfe207bfc248cf7ce571f6308ad3
wesm
pushed a commit
to wesm/arrow
that referenced
this pull request
Sep 7, 2018
Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#230 from majetideepak/PARQUET-846 and squashes the following commits: 9007986 [Deepak Majeti] PARQUET-846: CpuInfo::Init() is not thread safe Change-Id: I33bef39d6291dfe207bfc248cf7ce571f6308ad3
wesm
pushed a commit
to wesm/arrow
that referenced
this pull request
Sep 8, 2018
Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#230 from majetideepak/PARQUET-846 and squashes the following commits: 9007986 [Deepak Majeti] PARQUET-846: CpuInfo::Init() is not thread safe Change-Id: I33bef39d6291dfe207bfc248cf7ce571f6308ad3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The one downside of this patch is that HdfsFile handles don't get garbage-collected until the cyclic GC runs -- I tried to fix this but couldn't get it working. So bytes don't always get flushed to HDFS until
close()is called. The flush issue should be addressed on the C++ side