Repository: parquet-cpp Updated Branches: refs/heads/master eab9a7342 -> 46e40e20e
PARQUET-537: Ensure that LocalFileSource is properly closed. Author: Aliaksei Sandryhaila <[email protected]> Closes #68 from asandryh/PARQUET-537 and squashes the following commits: 18dca87 [Aliaksei Sandryhaila] Added a unit test. dfb7a0b [Aliaksei Sandryhaila] PARQUET-537: Ensure that LocalFileSource is properly closed. Project: http://git-wip-us.apache.org/repos/asf/parquet-cpp/repo Commit: http://git-wip-us.apache.org/repos/asf/parquet-cpp/commit/46e40e20 Tree: http://git-wip-us.apache.org/repos/asf/parquet-cpp/tree/46e40e20 Diff: http://git-wip-us.apache.org/repos/asf/parquet-cpp/diff/46e40e20 Branch: refs/heads/master Commit: 46e40e20e7afa1fe493ea927d5a9f689be71dbf8 Parents: eab9a73 Author: Aliaksei Sandryhaila <[email protected]> Authored: Tue Mar 1 17:33:51 2016 -0800 Committer: Julien Le Dem <[email protected]> Committed: Tue Mar 1 17:33:51 2016 -0800 ---------------------------------------------------------------------- src/parquet/file/reader-internal.cc | 4 ++++ src/parquet/file/reader-internal.h | 1 + src/parquet/file/reader.cc | 8 ++++++-- src/parquet/reader-test.cc | 35 +++++++++++++++++++++++++++++++- 4 files changed, 45 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader-internal.cc ---------------------------------------------------------------------- diff --git a/src/parquet/file/reader-internal.cc b/src/parquet/file/reader-internal.cc index 3d8c373..c571c72 100644 --- a/src/parquet/file/reader-internal.cc +++ b/src/parquet/file/reader-internal.cc @@ -215,6 +215,10 @@ void SerializedFile::Close() { source_->Close(); } +SerializedFile::~SerializedFile() { + Close(); +} + std::shared_ptr<RowGroupReader> SerializedFile::GetRowGroup(int i) { std::unique_ptr<SerializedRowGroup> contents(new SerializedRowGroup(source_.get(), &metadata_.row_groups[i])); http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader-internal.h ---------------------------------------------------------------------- diff --git a/src/parquet/file/reader-internal.h b/src/parquet/file/reader-internal.h index 08d4607..a398cb3 100644 --- a/src/parquet/file/reader-internal.h +++ b/src/parquet/file/reader-internal.h @@ -100,6 +100,7 @@ class SerializedFile : public ParquetFileReader::Contents { virtual int64_t num_rows() const; virtual int num_columns() const; virtual int num_row_groups() const; + virtual ~SerializedFile(); private: // This class takes ownership of the provided data source http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader.cc ---------------------------------------------------------------------- diff --git a/src/parquet/file/reader.cc b/src/parquet/file/reader.cc index fcbe453..8f639e9 100644 --- a/src/parquet/file/reader.cc +++ b/src/parquet/file/reader.cc @@ -65,7 +65,9 @@ RowGroupStatistics RowGroupReader::GetColumnStats(int i) const { // ParquetFileReader public API ParquetFileReader::ParquetFileReader() : schema_(nullptr) {} -ParquetFileReader::~ParquetFileReader() {} +ParquetFileReader::~ParquetFileReader() { + Close(); +} std::unique_ptr<ParquetFileReader> ParquetFileReader::OpenFile(const std::string& path, bool memory_map) { @@ -91,7 +93,9 @@ void ParquetFileReader::Open(std::unique_ptr<ParquetFileReader::Contents> conten } void ParquetFileReader::Close() { - contents_->Close(); + if (contents_) { + contents_->Close(); + } } int ParquetFileReader::num_row_groups() const { http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/reader-test.cc ---------------------------------------------------------------------- diff --git a/src/parquet/reader-test.cc b/src/parquet/reader-test.cc index 3ac1525..2c69ce1 100644 --- a/src/parquet/reader-test.cc +++ b/src/parquet/reader-test.cc @@ -16,6 +16,7 @@ // under the License. #include <gtest/gtest.h> +#include <fcntl.h> #include <cstdlib> #include <cstdint> #include <iostream> @@ -23,8 +24,10 @@ #include <string> #include "parquet/file/reader.h" +#include "parquet/file/reader-internal.h" #include "parquet/column/reader.h" #include "parquet/column/scanner.h" +#include "parquet/util/input.h" using std::string; @@ -50,7 +53,6 @@ class TestAllTypesPlain : public ::testing::Test { std::unique_ptr<ParquetFileReader> reader_; }; - TEST_F(TestAllTypesPlain, NoopConstructDestruct) { } @@ -121,4 +123,35 @@ TEST_F(TestAllTypesPlain, DebugPrintWorks) { ASSERT_GT(result.size(), 0); } + +class TestLocalFileSource : public ::testing::Test { + public: + void SetUp() { + std::string dir_string(data_dir); + + std::stringstream ss; + ss << dir_string << "/" << "alltypes_plain.parquet"; + + file.reset(new LocalFileSource()); + file->Open(ss.str()); + } + + void TearDown() {} + + protected: + std::unique_ptr<LocalFileSource> file; +}; + +TEST_F(TestLocalFileSource, FileClosedOnDestruction) { + int file_desc = file->file_descriptor(); + { + auto contents = SerializedFile::Open(std::move(file)); + std::unique_ptr<ParquetFileReader> result(new ParquetFileReader()); + result->Open(std::move(contents)); + } + ASSERT_EQ(-1, fcntl(file_desc, F_GETFD)); + ASSERT_EQ(EBADF, errno); +} + + } // namespace parquet_cpp
