aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian Tismer <tismer@stackless.com>2023-05-14 11:49:46 +0200
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2023-06-06 15:47:35 +0000
commit28ddc0e501c25d1142438dff27d04841bf423371 (patch)
tree2c2ecc68b75ba0a8e37dbff90169f1a92221d8fe
parent95857c8ca68720431a9af3969d2509dc6c367e1b (diff)
shiboken: Implement raising of unraisable exceptions correctly
TODO in another check-in: Control exec/run by an XML attribute [ChangeLog][PySide6] Unraisable exceptions are now handled by a handler on the stack or printed if impossible. Unraisable exceptions are stored in an error store if there is an error handler on the call stack that can handle it, otherwise it is printed immediately. We record the existence of an error handler in thread local storage, which solves thread problems automatically. Since exec and run functions completely block all handlers, we need to mark them as a special case. The overhead is minimal and uses constant memory per thread. Task-number: PYSIDE-2310 Task-number: PYSIDE-2321 Change-Id: Ic25a2ff8552baf6e132ad86a4ad0925375e7ea88 Fixes: PYSIDE-2335 Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io> (cherry picked from commit baedbe8353ee20eb7ac98e67b7985eb80959f14f) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--sources/pyside6/libpyside/signalmanager.cpp2
-rw-r--r--sources/shiboken6/generator/shiboken/cppgenerator.cpp34
-rw-r--r--sources/shiboken6/libshiboken/sbkerrors.cpp54
-rw-r--r--sources/shiboken6/libshiboken/sbkerrors.h18
4 files changed, 70 insertions, 38 deletions
diff --git a/sources/pyside6/libpyside/signalmanager.cpp b/sources/pyside6/libpyside/signalmanager.cpp
index b08ec24a5..efafb0aaf 100644
--- a/sources/pyside6/libpyside/signalmanager.cpp
+++ b/sources/pyside6/libpyside/signalmanager.cpp
@@ -415,7 +415,7 @@ int SignalManager::SignalManagerPrivate::qtPropertyMetacall(QObject *object,
: "Unknown property type '%s' of QObject '%s' used in fget with %R",
pp->d->typeName.constData(), metaObject->className(), excValue);
if (PyErr_Occurred())
- PyErr_Print();
+ Shiboken::Errors::storeErrorOrPrint();
Py_DECREF(excType);
Py_DECREF(excValue);
Py_XDECREF(excTraceback);
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index b589b3b3a..8b19ba9ad 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -1421,7 +1421,7 @@ void CppGenerator::writeVirtualMethodNative(TextStream &s,
s << "Shiboken::GilState gil;\n";
// Get out of virtual method call if someone already threw an error.
- s << "if (PyErr_Occurred())\n" << indent
+ s << "if (Shiboken::Errors::occurred())\n" << indent
<< returnStatement << '\n' << outdent;
// PYSIDE-1019: Add info about properties
@@ -1542,7 +1542,7 @@ void CppGenerator::writeVirtualMethodNative(TextStream &s,
s << "if (" << PYTHON_RETURN_VAR << ".isNull()) {\n" << indent
<< "// An error happened in python code!\n"
- << "PyErr_Print();\n"
+ << "Shiboken::Errors::storeErrorOrPrint();\n"
<< returnStatement << "\n" << outdent
<< "}\n";
@@ -2301,7 +2301,13 @@ void CppGenerator::writeMethodWrapperPreamble(TextStream &s,const OverloadData &
s << "Shiboken::AutoDecRef errInfo{};\n";
s << "static const char fullName[] = \"" << fullPythonFunctionName(rfunc, true)
- << "\";\nSBK_UNUSED(fullName)\n";
+ << "\";\nSBK_UNUSED(fullName)\n"
+ << "Shiboken::PythonContextMarker pcm;\n";
+ // PYSIDE-2335: Mark blocking calls like `exec` or `run` as such.
+ bool isBlockingFunction = rfunc->name() == u"exec"_s || rfunc->name() == u"exec_"_s
+ || rfunc->name() == u"run"_s;
+ if (isBlockingFunction)
+ s << "pcm.setBlocking();\n";
if (maxArgs > 0) {
s << "int overloadId = -1;\n"
@@ -2394,14 +2400,14 @@ void CppGenerator::writeConstructorWrapper(TextStream &s, const OverloadData &ov
QString pre = needsMetaObject ? u"bool usesPyMI = "_s : u""_s;
s << "\n// PyMI support\n"
<< pre << "Shiboken::callInheritedInit(self, args, kwds, fullName);\n"
- << "if (PyErr_Occurred())\n" << indent << errorReturn << outdent << "\n";
+ << "if (Shiboken::Errors::occurred())\n" << indent << errorReturn << outdent << "\n";
writeFunctionCalls(s, overloadData, classContext, errorReturn);
s << '\n';
const QString typeName = classContext.forSmartPointer()
? classContext.preciseType().cppSignature() : metaClass->qualifiedCppName();
- s << "if (PyErr_Occurred() || !Shiboken::Object::setCppPointer(sbkSelf, Shiboken::SbkType< ::"
+ s << "if (Shiboken::Errors::occurred() || !Shiboken::Object::setCppPointer(sbkSelf, Shiboken::SbkType< ::"
<< typeName << " >(), cptr)) {\n"
<< indent << "delete cptr;\n" << errorReturn << outdent
<< "}\n";
@@ -2521,7 +2527,7 @@ void CppGenerator::writeMethodWrapper(TextStream &s, const OverloadData &overloa
<< "PyObject *revOpMethod = PyObject_GetAttr(" << PYTHON_ARG << ", attrName);\n"
<< "if (revOpMethod && PyCallable_Check(revOpMethod)) {\n" << indent
<< PYTHON_RETURN_VAR << " = PyObject_CallFunction(revOpMethod, \"O\", self);\n"
- << "if (PyErr_Occurred() && (PyErr_ExceptionMatches(PyExc_NotImplementedError)"
+ << "if (Shiboken::Errors::occurred() && (PyErr_ExceptionMatches(PyExc_NotImplementedError)"
<< " || PyErr_ExceptionMatches(PyExc_AttributeError))) {\n" << indent
<< "PyErr_Clear();\n"
<< "Py_XDECREF(" << PYTHON_RETURN_VAR << ");\n"
@@ -2777,7 +2783,7 @@ void CppGenerator::writeFunctionReturnErrorCheckSection(TextStream &s,
ErrorReturn errorReturn,
bool hasReturnValue)
{
- s << "if (PyErr_Occurred()";
+ s << "if (Shiboken::Errors::occurred()";
if (hasReturnValue)
s << " || !" << PYTHON_RETURN_VAR;
s << ") {\n" << indent;
@@ -3382,7 +3388,7 @@ static void writeDeprecationWarning(TextStream &s,
s << cls->name() << "\", ";
// Check error in case "warning-as-error" is set.
s << '"' << func->signature().replace(u"::"_s, u"."_s) << "\");\n"
- << "if (PyErr_Occurred())\n" << indent << errorReturn << outdent;
+ << "if (Shiboken::Errors::occurred())\n" << indent << errorReturn << outdent;
}
void CppGenerator::writeSingleFunctionCall(TextStream &s,
@@ -3453,7 +3459,7 @@ void CppGenerator::writeSingleFunctionCall(TextStream &s,
int numRemovedArgs = OverloadData::numberOfRemovedArguments(func);
- s << "if (!PyErr_Occurred()) {\n" << indent;
+ s << "if (!Shiboken::Errors::occurred()) {\n" << indent;
writeMethodCall(s, func, context,
overloadData.pythonFunctionWrapperUsesListOfArguments(),
func->arguments().size() - numRemovedArgs, indirections, errorReturn);
@@ -5221,7 +5227,7 @@ void CppGenerator::writeGetterFunction(TextStream &s, const QPropertySpec &prope
s << "auto " << value << " = " << CPP_SELF_VAR << "->" << property.read() << "();\n"
<< "auto pyResult = ";
writeToPythonConversion(s, property.type(), context.metaClass(), value);
- s << ";\nif (PyErr_Occurred() || !pyResult) {\n" << indent
+ s << ";\nif (Shiboken::Errors::occurred() || !pyResult) {\n" << indent
<< "Py_XDECREF(pyResult);\nreturn {};\n" << outdent
<< "}\nreturn pyResult;\n" << outdent << "}\n\n";
}
@@ -5295,7 +5301,7 @@ void CppGenerator::writeSetterFunction(TextStream &s, const QPropertySpec &prope
s << "auto cppOut = " << CPP_SELF_VAR << "->" << property.read() << "();\n"
<< PYTHON_TO_CPP_VAR << "(pyIn, &cppOut);\n"
- << "if (PyErr_Occurred())\n" << indent
+ << "if (Shiboken::Errors::occurred())\n" << indent
<< "return -1;\n" << outdent
<< CPP_SELF_VAR << "->" << property.write() << "(cppOut);\n"
<< "return 0;\n" << outdent << "}\n\n";
@@ -5914,11 +5920,11 @@ void CppGenerator::writeFlagsBinaryOperator(TextStream &s, const AbstractMetaEnu
<< ">(int(PyLong_AsLong(self)));\n"
// PYSIDE-1436: Need to error check self as well because operators are used
// sometimes with swapped args.
- << "if (PyErr_Occurred())\n" << indent
+ << "if (Shiboken::Errors::occurred())\n" << indent
<< "return nullptr;\n" << outdent
<< "cppArg = static_cast<" << flagsEntry->originalName()
<< ">(int(PyLong_AsLong(" << PYTHON_ARG << ")));\n"
- << "if (PyErr_Occurred())\n" << indent
+ << "if (Shiboken::Errors::occurred())\n" << indent
<< "return nullptr;\n" << outdent
<< "cppResult = " << CPP_SELF_VAR << " " << cppOpName << " cppArg;\n"
<< "return ";
@@ -7030,7 +7036,7 @@ bool CppGenerator::finishGeneration()
}
}
- s << "\nif (PyErr_Occurred()) {\n" << indent
+ s << "\nif (Shiboken::Errors::occurred()) {\n" << indent
<< "PyErr_Print();\n"
<< "Py_FatalError(\"can't initialize module " << moduleName() << "\");\n"
<< outdent << "}\n";
diff --git a/sources/shiboken6/libshiboken/sbkerrors.cpp b/sources/shiboken6/libshiboken/sbkerrors.cpp
index 28ebd4864..1832624d5 100644
--- a/sources/shiboken6/libshiboken/sbkerrors.cpp
+++ b/sources/shiboken6/libshiboken/sbkerrors.cpp
@@ -4,9 +4,35 @@
#include "sbkerrors.h"
#include "sbkstring.h"
#include "helper.h"
+#include "gilstate.h"
namespace Shiboken
{
+
+// PYSIDE-2335: Track down if we can reach a Python error handler.
+// _pythonContextStack has always the current state of handler status
+// in its lowest bit.
+// Blocking calls like exec or run need to use `setBlocking`.
+static thread_local std::size_t _pythonContextStack{};
+
+PythonContextMarker::PythonContextMarker()
+{
+ // Shift history up and set lowest bit.
+ _pythonContextStack = (_pythonContextStack * 2) + 1;
+}
+
+PythonContextMarker::~PythonContextMarker()
+{
+ // Shift history down.
+ _pythonContextStack /= 2;
+}
+
+void PythonContextMarker::setBlocking()
+{
+ // Clear lowest bit.
+ _pythonContextStack = _pythonContextStack / 2 * 2;
+}
+
namespace Errors
{
@@ -73,35 +99,21 @@ struct ErrorStore {
PyObject *traceback;
};
-static ErrorStore savedError{};
+static thread_local ErrorStore savedError{};
-void storeError()
+void storeErrorOrPrint()
{
// This error happened in a function with no way to return an error state.
// Therefore, we handle the error when we are error checking, anyway.
- PyErr_Fetch(&savedError.type, &savedError.exc, &savedError.traceback);
- PyErr_NormalizeException(&savedError.type, &savedError.exc, &savedError.traceback);
-
- // In this error-free context, it is safe to call a string function.
- static PyObject *const msg = PyUnicode_FromString(" Note: This exception was delayed.");
- static PyObject *const _add_note = Shiboken::String::createStaticString("add_note");
- static bool hasAddNote = PyObject_HasAttr(PyExc_BaseException, _add_note);
- if (hasAddNote) {
- PyObject_CallMethodObjArgs(savedError.exc, _add_note, msg, nullptr);
- } else {
- PyObject *type, *exc, *traceback;
- PyErr_Format(PyExc_RuntimeError, "Delayed %s exception:",
- reinterpret_cast<PyTypeObject *>(savedError.type)->tp_name);
- PyErr_Fetch(&type, &exc, &traceback);
- PyException_SetContext(savedError.exc, exc);
- PyErr_NormalizeException(&type, &exc, &traceback);
- }
+ // But we do that only when we know that an error handler can pick it up.
+ if (_pythonContextStack & 1)
+ PyErr_Fetch(&savedError.type, &savedError.exc, &savedError.traceback);
+ else
+ PyErr_Print();
}
PyObject *occurred()
{
- // This error handler can be called in any Python context.
-
if (savedError.type) {
PyErr_Restore(savedError.type, savedError.exc, savedError.traceback);
savedError.type = nullptr;
diff --git a/sources/shiboken6/libshiboken/sbkerrors.h b/sources/shiboken6/libshiboken/sbkerrors.h
index 89b54c87a..6ff85f8e1 100644
--- a/sources/shiboken6/libshiboken/sbkerrors.h
+++ b/sources/shiboken6/libshiboken/sbkerrors.h
@@ -18,6 +18,20 @@
namespace Shiboken
{
+
+struct LIBSHIBOKEN_API PythonContextMarker
+{
+public:
+ PythonContextMarker(const PythonContextMarker &) = delete;
+ PythonContextMarker(PythonContextMarker &&) = delete;
+ PythonContextMarker &operator=(const PythonContextMarker &) = delete;
+ PythonContextMarker &operator=(PythonContextMarker &&) = delete;
+
+ explicit PythonContextMarker();
+ ~PythonContextMarker();
+ void setBlocking();
+};
+
namespace Errors
{
@@ -35,9 +49,9 @@ LIBSHIBOKEN_API void setWrongContainerType();
/// Report an error ASAP: Instead of printing, store for later re-raise.
/// This replaces `PyErr_Print`, which cannot report errors as exception.
/// To be used in contexts where raising errors is impossible.
-LIBSHIBOKEN_API void storeError();
+LIBSHIBOKEN_API void storeErrorOrPrint();
/// Handle an error as in PyErr_Occurred(), but also check for errors which
-/// were captured by `storeError`.
+/// were captured by `storeErrorOrPrint`.
/// To be used in normal error checks.
LIBSHIBOKEN_API PyObject *occurred();