diff --git a/util/env_windows.cc b/util/env_windows.cc index 449f564..a76d4d5 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -622,7 +622,10 @@ class WindowsEnv : public Env { } Status NewLogger(const std::string& filename, Logger** result) override { - std::FILE* fp = std::fopen(filename.c_str(), "w"); + // Use 'N' fopen() mode to open a non-inheritable file handle on Windows. + // Without this option, open handles in child processes may prevent the + // database from getting deleted. + std::FILE* fp = std::fopen(filename.c_str(), "wN"); if (fp == nullptr) { *result = nullptr; return WindowsError(filename, ::GetLastError()); diff --git a/util/env_windows_test.cc b/util/env_windows_test.cc index d6822d2..a2d04b4 100644 --- a/util/env_windows_test.cc +++ b/util/env_windows_test.cc @@ -2,12 +2,72 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. See the AUTHORS file for names of contributors. +#include + +#include + #include "gtest/gtest.h" #include "leveldb/env.h" #include "port/port.h" #include "util/env_windows_test_helper.h" #include "util/testutil.h" +namespace { + +// Returns void so the implementation can use ASSERT_EQ. +void GetOpenHandles(std::unordered_set* open_handles) { + constexpr int kHandleOffset = 4; + const HANDLE kHandleUpperBound = reinterpret_cast(1000 * kHandleOffset); + + for (HANDLE handle = nullptr; handle < kHandleUpperBound; reinterpret_cast(handle) += kHandleOffset) { + DWORD dwFlags; + if (!GetHandleInformation(handle, &dwFlags)) { + ASSERT_EQ(ERROR_INVALID_HANDLE, ::GetLastError()) + << "GetHandleInformation() should return ERROR_INVALID_HANDLE error on invalid handles"; + continue; + } + open_handles->insert(handle); + } +} + +// Returns void so the implementation can use ASSERT_GT. +void GetOpenedFileHandlesByFileName(const char* name, std::unordered_set* result_handles) { + std::unordered_set open_handles; + GetOpenHandles(&open_handles); + + for (auto handle_it = open_handles.begin(); handle_it != open_handles.end(); ) { + char handle_path[MAX_PATH]; + DWORD ret = ::GetFinalPathNameByHandleA(*handle_it, handle_path, sizeof handle_path, FILE_NAME_NORMALIZED); + if (ret != 0) { + ASSERT_GT(sizeof handle_path, ret) << "Path too long"; + + const char* last_backslash = std::strrchr(handle_path, '\\'); + ASSERT_NE(last_backslash, nullptr); + if (std::strcmp(name, last_backslash + 1) == 0) { + ++handle_it; + continue; // matched + } + } + + // otherwise remove it + handle_it = open_handles.erase(handle_it); + } + + *result_handles = std::move(open_handles); +} + +void CheckOpenedFileHandleNonInheritable(const char* name) { + std::unordered_set found_handles; + GetOpenedFileHandlesByFileName(name, &found_handles); + ASSERT_EQ(1, found_handles.size()); + + DWORD dwFlags; + ASSERT_TRUE(GetHandleInformation(*found_handles.begin(), &dwFlags)); + ASSERT_FALSE(dwFlags & HANDLE_FLAG_INHERIT); +} + +} // namespace + namespace leveldb { static const int kMMapLimit = 4; @@ -29,7 +89,7 @@ TEST_F(EnvWindowsTest, TestOpenOnRead) { ASSERT_LEVELDB_OK(env_->GetTestDirectory(&test_dir)); std::string test_file = test_dir + "/open_on_read.txt"; - FILE* f = std::fopen(test_file.c_str(), "w"); + FILE* f = std::fopen(test_file.c_str(), "wN"); ASSERT_TRUE(f != nullptr); const char kFileData[] = "abcdefghijklmnopqrstuvwxyz"; fputs(kFileData, f); @@ -55,6 +115,21 @@ TEST_F(EnvWindowsTest, TestOpenOnRead) { ASSERT_LEVELDB_OK(env_->RemoveFile(test_file)); } +TEST_F(EnvWindowsTest, TestHandleNotInheritedLogger) { + std::string test_dir; + ASSERT_LEVELDB_OK(env_->GetTestDirectory(&test_dir)); + const char kFileName[] = "handle_not_inherited_logger.txt"; + std::string file_path = test_dir + "/" + kFileName; + ASSERT_LEVELDB_OK(WriteStringToFile(env_, "0123456789", file_path)); + + leveldb::Logger* file = nullptr; + ASSERT_LEVELDB_OK(env_->NewLogger(file_path, &file)); + CheckOpenedFileHandleNonInheritable(kFileName); + delete file; + + ASSERT_LEVELDB_OK(env_->RemoveFile(file_path)); +} + } // namespace leveldb int main(int argc, char** argv) {