-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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-404: [Python] Fix segfault caused by HdfsClient getting closed before an HdfsFile #230
Conversation
…hen the last user-accessible client reference goes out of scope Change-Id: If97731c7cd663d89e418b4cdd713eb8f0add49e7
Change-Id: I4cf14f47203be94a8a95f5c7d9b780734a8cec67
def __dealloc__(self): | ||
self.file_handle.close() | ||
# try to avoid cyclic GC | ||
self.file_handle = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, let me try
Change-Id: I9f2b03e3f9e8677c89f144ff890edc3900431a40
Cool that worked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM
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
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
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
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
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
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