Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: properly translate C++ exception to Python exception when creating Python buffer from wrapped object #5324

Merged
merged 9 commits into from
Sep 2, 2024
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ set(PYBIND11_HEADERS
include/pybind11/detail/type_caster_base.h
include/pybind11/detail/typeid.h
include/pybind11/detail/value_and_holder.h
include/pybind11/detail/exception_translation.h
include/pybind11/attr.h
include/pybind11/buffer_info.h
include/pybind11/cast.h
Expand Down
15 changes: 14 additions & 1 deletion include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include <pybind11/attr.h>
#include <pybind11/options.h>

#include "exception_translation.h"

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)

Expand Down Expand Up @@ -581,7 +583,18 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla
return -1;
}
std::memset(view, 0, sizeof(Py_buffer));
buffer_info *info = tinfo->get_buffer(obj, tinfo->get_buffer_data);
buffer_info *info = nullptr;
try {
info = tinfo->get_buffer(obj, tinfo->get_buffer_data);
} catch (...) {
try_translate_exceptions();
vnlitvinov marked this conversation as resolved.
Show resolved Hide resolved
raise_from(PyExc_BufferError, "Error getting buffer");
return -1;
}
if (info == nullptr) {
pybind11_fail("FATAL UNEXPECTED SITUATION: tinfo->get_buffer() returned nullptr.");
}

if ((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && info->readonly) {
delete info;
// view->obj = nullptr; // Was just memset to 0, so not necessary
Expand Down
71 changes: 71 additions & 0 deletions include/pybind11/detail/exception_translation.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
pybind11/detail/exception_translation.h: means to translate C++ exceptions to Python exceptions

Copyright (c) 2024 The Pybind Development Team.

All rights reserved. Use of this source code is governed by a
BSD-style license that can be found in the LICENSE file.
*/

#pragma once

#include "common.h"
#include "internals.h"

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)

// Apply all the extensions translators from a list
// Return true if one of the translators completed without raising an exception
// itself. Return of false indicates that if there are other translators
// available, they should be tried.
inline bool apply_exception_translators(std::forward_list<ExceptionTranslator> &translators) {
auto last_exception = std::current_exception();

for (auto &translator : translators) {
try {
translator(last_exception);
return true;
} catch (...) {
last_exception = std::current_exception();
}
}
return false;
}

inline void try_translate_exceptions() {
/* When an exception is caught, give each registered exception
translator a chance to translate it to a Python exception. First
all module-local translators will be tried in reverse order of
registration. If none of the module-locale translators handle
the exception (or there are no module-locale translators) then
the global translators will be tried, also in reverse order of
registration.

A translator may choose to do one of the following:

- catch the exception and call py::set_error()
to set a standard (or custom) Python exception, or
- do nothing and let the exception fall through to the next translator, or
- delegate translation to the next translator by throwing a new type of exception.
*/

bool handled = with_internals([&](internals &internals) {
auto &local_exception_translators = get_local_internals().registered_exception_translators;
if (detail::apply_exception_translators(local_exception_translators)) {
return true;
}
auto &exception_translators = internals.registered_exception_translators;
if (detail::apply_exception_translators(exception_translators)) {
return true;
}
return false;
});

if (!handled) {
set_error(PyExc_SystemError, "Exception escaped from default exception translator!");
}
}

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
55 changes: 2 additions & 53 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
*/

#pragma once

#include "detail/class.h"
#include "detail/exception_translation.h"
#include "detail/init.h"
#include "attr.h"
#include "gil.h"
Expand Down Expand Up @@ -95,24 +95,6 @@ inline std::string replace_newlines_and_squash(const char *text) {
return result.substr(str_begin, str_range);
}

// Apply all the extensions translators from a list
// Return true if one of the translators completed without raising an exception
// itself. Return of false indicates that if there are other translators
// available, they should be tried.
inline bool apply_exception_translators(std::forward_list<ExceptionTranslator> &translators) {
auto last_exception = std::current_exception();

for (auto &translator : translators) {
try {
translator(last_exception);
return true;
} catch (...) {
last_exception = std::current_exception();
}
}
return false;
}

#if defined(_MSC_VER)
# define PYBIND11_COMPAT_STRDUP _strdup
#else
Expand Down Expand Up @@ -1038,40 +1020,7 @@ class cpp_function : public function {
throw;
#endif
} catch (...) {
/* When an exception is caught, give each registered exception
translator a chance to translate it to a Python exception. First
all module-local translators will be tried in reverse order of
registration. If none of the module-locale translators handle
the exception (or there are no module-locale translators) then
the global translators will be tried, also in reverse order of
registration.

A translator may choose to do one of the following:

- catch the exception and call py::set_error()
to set a standard (or custom) Python exception, or
- do nothing and let the exception fall through to the next translator, or
- delegate translation to the next translator by throwing a new type of exception.
*/

bool handled = with_internals([&](internals &internals) {
auto &local_exception_translators
= get_local_internals().registered_exception_translators;
if (detail::apply_exception_translators(local_exception_translators)) {
return true;
}
auto &exception_translators = internals.registered_exception_translators;
if (detail::apply_exception_translators(exception_translators)) {
return true;
}
return false;
});

if (handled) {
return nullptr;
}

set_error(PyExc_SystemError, "Exception escaped from default exception translator!");
try_translate_exceptions();
return nullptr;
}

Expand Down
1 change: 1 addition & 0 deletions tests/extra_python_package/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"include/pybind11/detail/type_caster_base.h",
"include/pybind11/detail/typeid.h",
"include/pybind11/detail/value_and_holder.h",
"include/pybind11/detail/exception_translation.h",
}

eigen_headers = {
Expand Down
12 changes: 12 additions & 0 deletions tests/test_buffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,18 @@ TEST_SUBMODULE(buffers, m) {
sizeof(float)});
});

class BrokenMatrix : public Matrix {
public:
BrokenMatrix(py::ssize_t rows, py::ssize_t cols) : Matrix(rows, cols) {}
void throw_runtime_error() { throw std::runtime_error("See PR #5324 for context."); }
};
py::class_<BrokenMatrix>(m, "BrokenMatrix", py::buffer_protocol())
.def(py::init<py::ssize_t, py::ssize_t>())
.def_buffer([](BrokenMatrix &m) {
m.throw_runtime_error();
return py::buffer_info();
});

// test_inherited_protocol
class SquareMatrix : public Matrix {
public:
Expand Down
7 changes: 7 additions & 0 deletions tests/test_buffers.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,10 @@ def test_buffer_docstring():
m.get_buffer_info.__doc__.strip()
== "get_buffer_info(arg0: Buffer) -> pybind11_tests.buffers.buffer_info"
)


def test_buffer_exception():
with pytest.raises(BufferError, match="Error getting buffer") as excinfo:
memoryview(m.BrokenMatrix(1, 1))
assert isinstance(excinfo.value.__cause__, RuntimeError)
assert "for context" in str(excinfo.value.__cause__)