From 66f7e44f3a6852753fa8b13beac480842c978d66 Mon Sep 17 00:00:00 2001
From: Ilia K <ki.stfu@gmail.com>
Date: Fri, 27 Nov 2020 19:02:28 +0300
Subject: [PATCH] Use 'N' mode in fopen() to disable handle inheritance on
Windows
Fixes #623
---
util/env_windows.cc | 5 ++-
util/env_windows_test.cc | 77 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 80 insertions(+), 2 deletions(-)
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 <windows.h>
+
+#include <unordered_set>
+
#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<HANDLE>* open_handles) {
+ constexpr int kHandleOffset = 4;
+ const HANDLE kHandleUpperBound = reinterpret_cast<HANDLE>(1000 * kHandleOffset);
+
+ for (HANDLE handle = nullptr; handle < kHandleUpperBound; reinterpret_cast<size_t&>(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<HANDLE>* result_handles) {
+ std::unordered_set<HANDLE> 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<HANDLE> 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) {