From 87ef595cce441304ca71552c2f405947df007be1 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 3 Aug 2017 16:20:28 +0100 Subject: [PATCH] For a block invoke to another thread, safe and restore * In future we could handle async exceptions by storing the exception information in a std::function derived object (instead of the separate ExceptionHandling that lives on the stack) and query it out in a new WaitForInvoke function maybe. Right now we just print the exception to the output log and abort the callback. --- qrenderdoc/Code/pyrenderdoc/PythonContext.cpp | 15 +++++ qrenderdoc/Code/pyrenderdoc/PythonContext.h | 4 ++ qrenderdoc/Code/pyrenderdoc/pyconversion.h | 57 ++++++++++++++----- .../Code/pyrenderdoc/pyrenderdoc_stub.cpp | 9 +++ qrenderdoc/Code/pyrenderdoc/qrenderdoc.i | 10 ++++ qrenderdoc/Code/pyrenderdoc/renderdoc.i | 10 ++-- 6 files changed, 88 insertions(+), 17 deletions(-) diff --git a/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp b/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp index fa99c72e0..cace6df30 100644 --- a/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp +++ b/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp @@ -829,3 +829,18 @@ extern "C" void HandleException(PyObject *global_handle) if(redirector->context) emit redirector->context->exception(typeStr, valueStr, frames); } + +extern "C" bool IsThreadBlocking(PyObject *global_handle) +{ + OutputRedirector *redirector = (OutputRedirector *)global_handle; + if(redirector && redirector->context) + return redirector->context->threadBlocking(); + return false; +} + +extern "C" void SetThreadBlocking(PyObject *global_handle, bool block) +{ + OutputRedirector *redirector = (OutputRedirector *)global_handle; + if(redirector && redirector->context) + return redirector->context->setThreadBlocking(block); +} \ No newline at end of file diff --git a/qrenderdoc/Code/pyrenderdoc/PythonContext.h b/qrenderdoc/Code/pyrenderdoc/PythonContext.h index d3d549ee1..c2268a6b2 100644 --- a/qrenderdoc/Code/pyrenderdoc/PythonContext.h +++ b/qrenderdoc/Code/pyrenderdoc/PythonContext.h @@ -89,6 +89,8 @@ public: } static QWidget *QWidgetFromPy(PyObject *widget); + void setThreadBlocking(bool block) { m_Block = block; } + bool threadBlocking() { return m_Block; } void abort() { m_Abort = true; } bool shouldAbort() { return m_Abort; } QString currentFile() { return location.file; } @@ -128,6 +130,8 @@ private: bool m_Abort = false; + bool m_Block = false; + static PyObject *QtObjectToPython(PyObject *self, const char *typeName, QObject *object); QTimer *outputTicker; diff --git a/qrenderdoc/Code/pyrenderdoc/pyconversion.h b/qrenderdoc/Code/pyrenderdoc/pyconversion.h index 34cac4c87..1fcbab45a 100644 --- a/qrenderdoc/Code/pyrenderdoc/pyconversion.h +++ b/qrenderdoc/Code/pyrenderdoc/pyconversion.h @@ -800,19 +800,29 @@ PyObject *ConvertToPy(PyObject *self, const T &in) // this is defined elsewhere for managing the opaque global_handle object extern "C" PyThreadState *GetExecutingThreadState(PyObject *global_handle); extern "C" void HandleException(PyObject *global_handle); +extern "C" bool IsThreadBlocking(PyObject *global_handle); +extern "C" void SetThreadBlocking(PyObject *global_handle, bool block); + +struct ExceptionHandling +{ + bool failFlag = false; + PyObject *exObj = NULL; + PyObject *valueObj = NULL; + PyObject *tracebackObj = NULL; +}; // this function handles failures in callback functions. If we're synchronously calling the callback // from within an execute scope, then we can assign to failflag and let the error propagate upwards. // If we're not, then the callback is being executed on another thread with no knowledge of python, // so we need to use the global handle to try and emit the exception through the context. None of // this is multi-threaded because we're inside the GIL at all times -inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag) +inline void HandleCallbackFailure(PyObject *global_handle, ExceptionHandling &exHandle) { // if there's no global handle assume we are not running in the usual environment, so there are no // external-to-python threads if(!global_handle) { - fail_flag = true; + exHandle.failFlag = true; return; } @@ -822,7 +832,25 @@ inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag) // we are executing synchronously, set the flag and return if(current == executing) { - fail_flag = true; + exHandle.failFlag = true; + return; + } + + // if we have the blocking flag set, then we may be on another thread but we can still propagate + // up the error + if(IsThreadBlocking(global_handle)) + { + exHandle.failFlag = true; + + // we need to rethrow the exception to that thread, so fetch (and clear it) on this thread. + // + // Note that the exception can only propagate up to one place. However since we know that python + // is inherently single threaded, so if we're doing this blocking funciton call on another + // thread then we *know* there isn't python further up the stack. Therefore we're safe to + // swallow the exception here (since there's nowhere for it to bubble up to anyway) and rethrow + // on the python thread. + PyErr_Fetch(&exHandle.exObj, &exHandle.valueObj, &exHandle.tracebackObj); + return; } @@ -832,7 +860,8 @@ inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag) } template -inline T get_return(const char *funcname, PyObject *result, PyObject *global_handle, bool &failflag) +inline T get_return(const char *funcname, PyObject *result, PyObject *global_handle, + ExceptionHandling &exHandle) { T val = T(); @@ -840,7 +869,7 @@ inline T get_return(const char *funcname, PyObject *result, PyObject *global_han if(!SWIG_IsOK(res)) { - HandleCallbackFailure(global_handle, failflag); + HandleCallbackFailure(global_handle, exHandle); PyErr_Format(PyExc_TypeError, "Expected a '%s' for return value of callback in %s", TypeName(), funcname); @@ -852,7 +881,8 @@ inline T get_return(const char *funcname, PyObject *result, PyObject *global_han } template <> -inline void get_return(const char *funcname, PyObject *result, PyObject *global_handle, bool &failflag) +inline void get_return(const char *funcname, PyObject *result, PyObject *global_handle, + ExceptionHandling &exHandle) { Py_XDECREF(result); } @@ -897,22 +927,23 @@ struct varfunc } ~varfunc() { Py_XDECREF(args); } - rettype call(const char *funcname, PyObject *func, PyObject *global_handle, bool &failflag) + rettype call(const char *funcname, PyObject *func, PyObject *global_handle, + ExceptionHandling &exHandle) { if(!func || func == Py_None || !PyCallable_Check(func) || !args) { - HandleCallbackFailure(global_handle, failflag); + HandleCallbackFailure(global_handle, exHandle); return rettype(); } PyObject *result = PyObject_Call(func, args, 0); if(result == NULL) - HandleCallbackFailure(global_handle, failflag); + HandleCallbackFailure(global_handle, exHandle); Py_DECREF(args); - return get_return(funcname, result, global_handle, failflag); + return get_return(funcname, result, global_handle, exHandle); } int currentarg = 0; @@ -939,7 +970,7 @@ struct ScopedFuncCall }; template -funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, bool &failflag) +funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, ExceptionHandling &exHandle) { // add a reference to the global object so it stays alive while we execute, in case this is an // async call @@ -949,11 +980,11 @@ funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, bool if(globals) global_internal_handle = PyDict_GetItemString(globals, "_renderdoc_internal"); - return [global_internal_handle, self, funcname, func, &failflag](auto... param) { + return [global_internal_handle, self, funcname, func, &exHandle](auto... param) { ScopedFuncCall gil(global_internal_handle); varfunc f(self, funcname, param...); - return f.call(funcname, func, global_internal_handle, failflag); + return f.call(funcname, func, global_internal_handle, exHandle); }; } diff --git a/qrenderdoc/Code/pyrenderdoc/pyrenderdoc_stub.cpp b/qrenderdoc/Code/pyrenderdoc/pyrenderdoc_stub.cpp index 930b70ce4..10af8b92b 100644 --- a/qrenderdoc/Code/pyrenderdoc/pyrenderdoc_stub.cpp +++ b/qrenderdoc/Code/pyrenderdoc/pyrenderdoc_stub.cpp @@ -32,3 +32,12 @@ extern "C" PyThreadState *GetExecutingThreadState(PyObject *global_handle) extern "C" void HandleException(PyObject *global_handle) { } + +extern "C" bool IsThreadBlocking(PyObject *global_handle) +{ + return false; +} + +extern "C" void SetThreadBlocking(PyObject *global_handle, bool block) +{ +} diff --git a/qrenderdoc/Code/pyrenderdoc/qrenderdoc.i b/qrenderdoc/Code/pyrenderdoc/qrenderdoc.i index b4e979430..4d7c08d53 100644 --- a/qrenderdoc/Code/pyrenderdoc/qrenderdoc.i +++ b/qrenderdoc/Code/pyrenderdoc/qrenderdoc.i @@ -77,9 +77,19 @@ CONTAINER_TYPEMAPS(QMap) %extend IReplayManager { void BlockInvoke(InvokeCallback m) { + PyObject *global_internal_handle = NULL; + + PyObject *globals = PyEval_GetGlobals(); + if(globals) + global_internal_handle = PyDict_GetItemString(globals, "_renderdoc_internal"); + + SetThreadBlocking(global_internal_handle, true); + Py_BEGIN_ALLOW_THREADS $self->BlockInvoke(m); Py_END_ALLOW_THREADS + + SetThreadBlocking(global_internal_handle, false); } }; diff --git a/qrenderdoc/Code/pyrenderdoc/renderdoc.i b/qrenderdoc/Code/pyrenderdoc/renderdoc.i index b05ad9316..ea82a33c9 100644 --- a/qrenderdoc/Code/pyrenderdoc/renderdoc.i +++ b/qrenderdoc/Code/pyrenderdoc/renderdoc.i @@ -53,12 +53,14 @@ CONTAINER_TYPEMAPS(rdctype::array) %typemap(in, fragment="pyconvert") std::function { PyObject *func = $input; - failed$argnum = false; - $1 = ConvertFunc<$1_ltype>(self, "$symname", func, failed$argnum); + $1 = ConvertFunc<$1_ltype>(self, "$symname", func, exHandle$argnum); } -%typemap(argout) std::function (bool failed) { - if(failed) SWIG_fail; +%typemap(argout) std::function (ExceptionHandling exHandle) { + if(exHandle.failFlag) { + PyErr_Restore(exHandle.exObj, exHandle.valueObj, exHandle.tracebackObj); + SWIG_fail; + } } // ignore some operators SWIG doesn't have to worry about