1
0
Fork 0

fix potential duplicate closing of typed buffer (#3587)

This commit is contained in:
Jan 2017-11-07 10:27:51 +01:00 committed by Frank Celler
parent f6a90c4879
commit 151c717d22
3 changed files with 18 additions and 5 deletions

View File

@ -42,7 +42,7 @@ namespace pregel {
template <typename T> template <typename T>
struct TypedBuffer { struct TypedBuffer {
/// close file (see close() ) /// close file (see close() )
virtual ~TypedBuffer(){}; virtual ~TypedBuffer() {}
TypedBuffer() : _ptr(nullptr) {} TypedBuffer() : _ptr(nullptr) {}
/// @brief return whether the datafile is a physical file (true) or an /// @brief return whether the datafile is a physical file (true) or an
@ -112,7 +112,7 @@ template <typename T>
class MappedFileBuffer : public TypedBuffer<T> { class MappedFileBuffer : public TypedBuffer<T> {
public: public:
#ifdef TRI_HAVE_ANONYMOUS_MMAP #ifdef TRI_HAVE_ANONYMOUS_MMAP
MappedFileBuffer(size_t entries) : _size(entries) { explicit MappedFileBuffer(size_t entries) : _size(entries) {
#ifdef TRI_MMAP_ANONYMOUS #ifdef TRI_MMAP_ANONYMOUS
// fd -1 is required for "real" anonymous regions // fd -1 is required for "real" anonymous regions
_fd = -1; _fd = -1;
@ -156,7 +156,7 @@ class MappedFileBuffer : public TypedBuffer<T> {
// ptr); // ptr);
} }
#else #else
MappedFileBuffer(size_t entries) : _size(entries) { explicit MappedFileBuffer(size_t entries) : _size(entries) {
double tt = TRI_microtime(); double tt = TRI_microtime();
std::string file = "pregel_" + std::to_string((uint64_t)tt) + ".mmap"; std::string file = "pregel_" + std::to_string((uint64_t)tt) + ".mmap";
std::string filename = FileUtils::buildFilename(TRI_GetTempPath(), file); std::string filename = FileUtils::buildFilename(TRI_GetTempPath(), file);
@ -223,6 +223,11 @@ class MappedFileBuffer : public TypedBuffer<T> {
/// close file /// close file
void close() override { void close() override {
if (this->_ptr == nullptr) {
// already closed or not opened
return;
}
int res = TRI_UNMMFile(this->_ptr, _mappedSize, _fd, &_mmHandle); int res = TRI_UNMMFile(this->_ptr, _mappedSize, _fd, &_mmHandle);
if (res != TRI_ERROR_NO_ERROR) { if (res != TRI_ERROR_NO_ERROR) {
// leave file open here as it will still be memory-mapped // leave file open here as it will still be memory-mapped

View File

@ -256,7 +256,16 @@ int TRI_UNMMFile(void* memoryAddress, size_t numOfBytesToUnMap,
// UnmapViewOfFile: If the function succeeds, the return value is nonzero. // UnmapViewOfFile: If the function succeeds, the return value is nonzero.
bool ok = (UnmapViewOfFile(memoryAddress) != 0); bool ok = (UnmapViewOfFile(memoryAddress) != 0);
ok = (CloseHandle(*mmHandle) && ok); if (!ok) {
DWORD errorCode = GetLastError();
LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "UnmapViewOfFile returned an error: " << errorCode;
}
if (CloseHandle(*mmHandle) == 0) {
DWORD errorCode = GetLastError();
LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "CloseHandle returned an error: " << errorCode;
ok = false;
}
if (!ok) { if (!ok) {
return TRI_ERROR_SYS_ERROR; return TRI_ERROR_SYS_ERROR;

View File

@ -74,4 +74,3 @@ TEST_CASE("tst_pregel1", "[pregel][mmap]") {
REQUIRE(mapped.data() == nullptr); REQUIRE(mapped.data() == nullptr);
} }
/* end of georeg.cpp */