diff options
author | Christian Tismer <tismer@stackless.com> | 2023-05-14 11:49:46 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-06-06 15:47:35 +0000 |
commit | 28ddc0e501c25d1142438dff27d04841bf423371 (patch) | |
tree | 2c2ecc68b75ba0a8e37dbff90169f1a92221d8fe | |
parent | 95857c8ca68720431a9af3969d2509dc6c367e1b (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.cpp | 2 | ||||
-rw-r--r-- | sources/shiboken6/generator/shiboken/cppgenerator.cpp | 34 | ||||
-rw-r--r-- | sources/shiboken6/libshiboken/sbkerrors.cpp | 54 | ||||
-rw-r--r-- | sources/shiboken6/libshiboken/sbkerrors.h | 18 |
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(); |