Skip to content

Commit

Permalink
Add win32 access violation handler to buffer manager
Browse files Browse the repository at this point in the history
  • Loading branch information
benjaminwinger committed Sep 15, 2023
1 parent 49850c8 commit 4417cda
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ if(MSVC)
add_compile_definitions(_AMD64_)
# Non-english windows system may use other encodings other than utf-8 (e.g. Chinese use GBK).
add_compile_options("/utf-8")
# Enables support for custom hardware exception handling
add_compile_options("/EHa")
# Remove the default to avoid warnings
STRING(REPLACE "/EHsc" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
STRING(REPLACE "/EHs" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
endif()
if(CMAKE_BUILD_TYPE MATCHES Release)
if(MSVC)
Expand Down
4 changes: 4 additions & 0 deletions src/include/storage/buffer_manager/vm_region.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ class VMRegion {
// Use `MADV_DONTNEED` to release physical memory associated with this frame.
void releaseFrame(common::frame_idx_t frameIdx);

// Returns true if the memory address is within the reserved virtual memory region
bool contains(const uint8_t* address) const {
return address >= region && address < region + getMaxRegionSize();
}
#ifdef _WIN32
uint8_t* getFrame(common::frame_idx_t frameIdx);
#else
Expand Down
74 changes: 71 additions & 3 deletions src/storage/buffer_manager/buffer_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
#include "common/exception/buffer_manager.h"
#include "spdlog/spdlog.h"

#if defined(_WIN32)
#include <exception>

#include <eh.h>
#endif

using namespace kuzu::common;

namespace kuzu {
Expand Down Expand Up @@ -108,23 +114,85 @@ uint8_t* BufferManager::pin(
}
}

#if defined(WIN32)
class AccessViolation : public std::exception {
public:
AccessViolation(const uint8_t* location) : location{location} {}

const uint8_t* location;
};

class ScopedTranslator {
const _se_translator_function old;

public:
ScopedTranslator(_se_translator_function newTranslator)
: old{_set_se_translator(newTranslator)} {}
~ScopedTranslator() { _set_se_translator(old); }
};

void handleAccessViolation(unsigned int exceptionCode, PEXCEPTION_POINTERS exceptionRecord) {
if (exceptionCode == EXCEPTION_ACCESS_VIOLATION
// exception was from a read
&& exceptionRecord->ExceptionRecord->ExceptionInformation[0] == 0) [[likely]] {
throw AccessViolation(
(const uint8_t*)exceptionRecord->ExceptionRecord->ExceptionInformation[1]);
}
// Needs to not be an Exception so that it can't be caught by regular exception handling
// And is seems like throwing integer error codes is treated similarly to hardware
// exceptions with /EHa
throw exceptionCode;
}
#endif

// Returns true if the function completes successfully
inline bool try_func(const std::function<void(uint8_t*)>& func, uint8_t* frame,
const std::vector<std::unique_ptr<VMRegion>>& vmRegions, common::PageSizeClass pageSizeClass) {
#if defined(_WIN32)
try {
#endif
func(frame);
#if defined(_WIN32)
} catch (AccessViolation& exc) {
// If we encounter an acess violation within the VM region,
// the page was decomitted by another thread
// and is no longer valid memory
if (vmRegions[pageSizeClass]->contains(exc.location)) {
return false;
} else {
throw EXCEPTION_ACCESS_VIOLATION;
}
}
#endif
return true;
}

void BufferManager::optimisticRead(
BMFileHandle& fileHandle, page_idx_t pageIdx, const std::function<void(uint8_t*)>& func) {
auto pageState = fileHandle.getPageState(pageIdx);
#if defined(_WIN32)
// Change the Structured Exception handling just for the scope of this function
auto translator = ScopedTranslator(handleAccessViolation);
#endif
while (true) {
auto currStateAndVersion = pageState->getStateAndVersion();
switch (PageState::getState(currStateAndVersion)) {
case PageState::UNLOCKED: {
func(getFrame(fileHandle, pageIdx));
if (!try_func(func, getFrame(fileHandle, pageIdx), vmRegions,
fileHandle.getPageSizeClass())) {
continue;
}
if (pageState->getStateAndVersion() == currStateAndVersion) {
return;
}
} break;
case PageState::MARKED: {
// If the page is marked, we try to switch to unlocked. If we succeed, we read the page.
if (pageState->tryClearMark(currStateAndVersion)) {
func(getFrame(fileHandle, pageIdx));
return;
if (try_func(func, getFrame(fileHandle, pageIdx), vmRegions,
fileHandle.getPageSizeClass())) {
return;
}
}
} break;
case PageState::EVICTED: {
Expand Down

0 comments on commit 4417cda

Please sign in to comment.