Skip to content

Commit 87ef595

Browse files
committed
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.
1 parent 1c93ee9 commit 87ef595

File tree

6 files changed

+88
-17
lines changed

6 files changed

+88
-17
lines changed

qrenderdoc/Code/pyrenderdoc/PythonContext.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,3 +829,18 @@ extern "C" void HandleException(PyObject *global_handle)
829829
if(redirector->context)
830830
emit redirector->context->exception(typeStr, valueStr, frames);
831831
}
832+
833+
extern "C" bool IsThreadBlocking(PyObject *global_handle)
834+
{
835+
OutputRedirector *redirector = (OutputRedirector *)global_handle;
836+
if(redirector && redirector->context)
837+
return redirector->context->threadBlocking();
838+
return false;
839+
}
840+
841+
extern "C" void SetThreadBlocking(PyObject *global_handle, bool block)
842+
{
843+
OutputRedirector *redirector = (OutputRedirector *)global_handle;
844+
if(redirector && redirector->context)
845+
return redirector->context->setThreadBlocking(block);
846+
}

qrenderdoc/Code/pyrenderdoc/PythonContext.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ class PythonContext : public QObject
8989
}
9090
static QWidget *QWidgetFromPy(PyObject *widget);
9191

92+
void setThreadBlocking(bool block) { m_Block = block; }
93+
bool threadBlocking() { return m_Block; }
9294
void abort() { m_Abort = true; }
9395
bool shouldAbort() { return m_Abort; }
9496
QString currentFile() { return location.file; }
@@ -128,6 +130,8 @@ public slots:
128130

129131
bool m_Abort = false;
130132

133+
bool m_Block = false;
134+
131135
static PyObject *QtObjectToPython(PyObject *self, const char *typeName, QObject *object);
132136

133137
QTimer *outputTicker;

qrenderdoc/Code/pyrenderdoc/pyconversion.h

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -800,19 +800,29 @@ PyObject *ConvertToPy(PyObject *self, const T &in)
800800
// this is defined elsewhere for managing the opaque global_handle object
801801
extern "C" PyThreadState *GetExecutingThreadState(PyObject *global_handle);
802802
extern "C" void HandleException(PyObject *global_handle);
803+
extern "C" bool IsThreadBlocking(PyObject *global_handle);
804+
extern "C" void SetThreadBlocking(PyObject *global_handle, bool block);
805+
806+
struct ExceptionHandling
807+
{
808+
bool failFlag = false;
809+
PyObject *exObj = NULL;
810+
PyObject *valueObj = NULL;
811+
PyObject *tracebackObj = NULL;
812+
};
803813

804814
// this function handles failures in callback functions. If we're synchronously calling the callback
805815
// from within an execute scope, then we can assign to failflag and let the error propagate upwards.
806816
// If we're not, then the callback is being executed on another thread with no knowledge of python,
807817
// so we need to use the global handle to try and emit the exception through the context. None of
808818
// this is multi-threaded because we're inside the GIL at all times
809-
inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag)
819+
inline void HandleCallbackFailure(PyObject *global_handle, ExceptionHandling &exHandle)
810820
{
811821
// if there's no global handle assume we are not running in the usual environment, so there are no
812822
// external-to-python threads
813823
if(!global_handle)
814824
{
815-
fail_flag = true;
825+
exHandle.failFlag = true;
816826
return;
817827
}
818828

@@ -822,7 +832,25 @@ inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag)
822832
// we are executing synchronously, set the flag and return
823833
if(current == executing)
824834
{
825-
fail_flag = true;
835+
exHandle.failFlag = true;
836+
return;
837+
}
838+
839+
// if we have the blocking flag set, then we may be on another thread but we can still propagate
840+
// up the error
841+
if(IsThreadBlocking(global_handle))
842+
{
843+
exHandle.failFlag = true;
844+
845+
// we need to rethrow the exception to that thread, so fetch (and clear it) on this thread.
846+
//
847+
// Note that the exception can only propagate up to one place. However since we know that python
848+
// is inherently single threaded, so if we're doing this blocking funciton call on another
849+
// thread then we *know* there isn't python further up the stack. Therefore we're safe to
850+
// swallow the exception here (since there's nowhere for it to bubble up to anyway) and rethrow
851+
// on the python thread.
852+
PyErr_Fetch(&exHandle.exObj, &exHandle.valueObj, &exHandle.tracebackObj);
853+
826854
return;
827855
}
828856

@@ -832,15 +860,16 @@ inline void HandleCallbackFailure(PyObject *global_handle, bool &fail_flag)
832860
}
833861

834862
template <typename T>
835-
inline T get_return(const char *funcname, PyObject *result, PyObject *global_handle, bool &failflag)
863+
inline T get_return(const char *funcname, PyObject *result, PyObject *global_handle,
864+
ExceptionHandling &exHandle)
836865
{
837866
T val = T();
838867

839868
int res = ConvertToPy(result, val);
840869

841870
if(!SWIG_IsOK(res))
842871
{
843-
HandleCallbackFailure(global_handle, failflag);
872+
HandleCallbackFailure(global_handle, exHandle);
844873

845874
PyErr_Format(PyExc_TypeError, "Expected a '%s' for return value of callback in %s",
846875
TypeName<T>(), funcname);
@@ -852,7 +881,8 @@ inline T get_return(const char *funcname, PyObject *result, PyObject *global_han
852881
}
853882

854883
template <>
855-
inline void get_return(const char *funcname, PyObject *result, PyObject *global_handle, bool &failflag)
884+
inline void get_return(const char *funcname, PyObject *result, PyObject *global_handle,
885+
ExceptionHandling &exHandle)
856886
{
857887
Py_XDECREF(result);
858888
}
@@ -897,22 +927,23 @@ struct varfunc
897927
}
898928

899929
~varfunc() { Py_XDECREF(args); }
900-
rettype call(const char *funcname, PyObject *func, PyObject *global_handle, bool &failflag)
930+
rettype call(const char *funcname, PyObject *func, PyObject *global_handle,
931+
ExceptionHandling &exHandle)
901932
{
902933
if(!func || func == Py_None || !PyCallable_Check(func) || !args)
903934
{
904-
HandleCallbackFailure(global_handle, failflag);
935+
HandleCallbackFailure(global_handle, exHandle);
905936
return rettype();
906937
}
907938

908939
PyObject *result = PyObject_Call(func, args, 0);
909940

910941
if(result == NULL)
911-
HandleCallbackFailure(global_handle, failflag);
942+
HandleCallbackFailure(global_handle, exHandle);
912943

913944
Py_DECREF(args);
914945

915-
return get_return<rettype>(funcname, result, global_handle, failflag);
946+
return get_return<rettype>(funcname, result, global_handle, exHandle);
916947
}
917948

918949
int currentarg = 0;
@@ -939,7 +970,7 @@ struct ScopedFuncCall
939970
};
940971

941972
template <typename funcType>
942-
funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, bool &failflag)
973+
funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, ExceptionHandling &exHandle)
943974
{
944975
// add a reference to the global object so it stays alive while we execute, in case this is an
945976
// async call
@@ -949,11 +980,11 @@ funcType ConvertFunc(PyObject *self, const char *funcname, PyObject *func, bool
949980
if(globals)
950981
global_internal_handle = PyDict_GetItemString(globals, "_renderdoc_internal");
951982

952-
return [global_internal_handle, self, funcname, func, &failflag](auto... param) {
983+
return [global_internal_handle, self, funcname, func, &exHandle](auto... param) {
953984
ScopedFuncCall gil(global_internal_handle);
954985

955986
varfunc<typename funcType::result_type, decltype(param)...> f(self, funcname, param...);
956-
return f.call(funcname, func, global_internal_handle, failflag);
987+
return f.call(funcname, func, global_internal_handle, exHandle);
957988
};
958989
}
959990

qrenderdoc/Code/pyrenderdoc/pyrenderdoc_stub.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,12 @@ extern "C" PyThreadState *GetExecutingThreadState(PyObject *global_handle)
3232
extern "C" void HandleException(PyObject *global_handle)
3333
{
3434
}
35+
36+
extern "C" bool IsThreadBlocking(PyObject *global_handle)
37+
{
38+
return false;
39+
}
40+
41+
extern "C" void SetThreadBlocking(PyObject *global_handle, bool block)
42+
{
43+
}

qrenderdoc/Code/pyrenderdoc/qrenderdoc.i

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,19 @@ CONTAINER_TYPEMAPS(QMap)
7777

7878
%extend IReplayManager {
7979
void BlockInvoke(InvokeCallback m) {
80+
PyObject *global_internal_handle = NULL;
81+
82+
PyObject *globals = PyEval_GetGlobals();
83+
if(globals)
84+
global_internal_handle = PyDict_GetItemString(globals, "_renderdoc_internal");
85+
86+
SetThreadBlocking(global_internal_handle, true);
87+
8088
Py_BEGIN_ALLOW_THREADS
8189
$self->BlockInvoke(m);
8290
Py_END_ALLOW_THREADS
91+
92+
SetThreadBlocking(global_internal_handle, false);
8393
}
8494
};
8595

qrenderdoc/Code/pyrenderdoc/renderdoc.i

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,14 @@ CONTAINER_TYPEMAPS(rdctype::array)
5353

5454
%typemap(in, fragment="pyconvert") std::function {
5555
PyObject *func = $input;
56-
failed$argnum = false;
57-
$1 = ConvertFunc<$1_ltype>(self, "$symname", func, failed$argnum);
56+
$1 = ConvertFunc<$1_ltype>(self, "$symname", func, exHandle$argnum);
5857
}
5958

60-
%typemap(argout) std::function (bool failed) {
61-
if(failed) SWIG_fail;
59+
%typemap(argout) std::function (ExceptionHandling exHandle) {
60+
if(exHandle.failFlag) {
61+
PyErr_Restore(exHandle.exObj, exHandle.valueObj, exHandle.tracebackObj);
62+
SWIG_fail;
63+
}
6264
}
6365

6466
// ignore some operators SWIG doesn't have to worry about

0 commit comments

Comments
 (0)