Add win32 access violation handler to buffer manager #2035
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Windows will produce an access violation hardware exception (a.k.a. segfault) when attempting to read from de-committed memory, something which may occur in the buffer manager if another thread releases memory which was about to be read from. On linux/unix this isn't an issue since the kernel will allow the memory to be accessed and just provide empty data.
I've worked around this by enabling the
/EHa
compiler flag, which turns on custom exception handling for hardware exceptions. There are two downsides to this:catch (...)
will now catch hardware exceptions on windows, which is bad (note that the...
is explicit here). There are no instances of this in our code, but there are a few inthird_party
. One in spdlog which is just used to log information and re-throw (which should be fine), one in antlr4_runtime in a function we are not using, and a few in pybind11, which might affect the python library on windows./EHs
(only catches C++ exceptions) it can optimize away try/catch blocks when there are no exceptions that could be thrown. I don't think this is very significant.The custom exception handler raises a custom exception if the exception is a segfault, and the memory location of the segfault is checked to make sure it occurred within the buffer manager's reserved virtual memory to avoid catching other segfaults within the same scope (silently ignoring those would be a major issue).
The one thing I'm not sure about, is that the use of
catch (...)
in pybind11 seems to be in the code that handles translating c++ exceptions into python exceptions (i.e. it will cover all exceptions thrown by our c++ code), and it looks like it's turning arbitrary non-standard exceptions into a python RuntimeError, which seems like a bad way of reporting hardware exceptions like segfaults in the C++ extension.