diff options
author | Christian Tismer <tismer@stackless.com> | 2020-03-30 19:46:41 +0200 |
---|---|---|
committer | Christian Tismer <tismer@stackless.com> | 2020-04-02 15:11:06 +0200 |
commit | fe77dce7a7e713456454e5c0a071f7cc70261b98 (patch) | |
tree | bd904887a1b53856f9b95f521e8c1debe1d13b25 | |
parent | 79c74e1999d2b29bc918f6d42079c34174bb137d (diff) |
shiboken: Fix dict access without GIL
In PYSIDE-803 we used an optimization that accessed
a dictionary without holding the GIL. This turned out to be not
correct, because PyDict_GetItem works with thread state
to maintain the global error variables.
PyDict_GetItemWithErrors can be used instead in a way that
allows releasing the GIL.
Task-number: PYSIDE-803
Task-number: PYSIDE-813
Change-Id: Ifb0cbb20c21ca9c8b3d099fff1db5410eb6824b4
Reviewed-by: Christian Tismer <tismer@stackless.com>
-rw-r--r-- | sources/pyside2/libpyside/signalmanager.cpp | 5 | ||||
-rw-r--r-- | sources/pyside2/libpyside/signalmanager.h | 1 | ||||
-rw-r--r-- | sources/shiboken2/libshiboken/pep384impl.cpp | 45 | ||||
-rw-r--r-- | sources/shiboken2/libshiboken/pep384impl.h | 11 | ||||
-rw-r--r-- | sources/shiboken2/libshiboken/pep384impl_doc.rst | 10 |
5 files changed, 71 insertions, 1 deletions
diff --git a/sources/pyside2/libpyside/signalmanager.cpp b/sources/pyside2/libpyside/signalmanager.cpp index 01b347a3d..0fe0d0092 100644 --- a/sources/pyside2/libpyside/signalmanager.cpp +++ b/sources/pyside2/libpyside/signalmanager.cpp @@ -559,7 +559,10 @@ static MetaObjectBuilder *metaBuilderFromDict(PyObject *dict) if (!dict || !PyDict_Contains(dict, metaObjectAttr)) return nullptr; - PyObject *pyBuilder = PyDict_GetItem(dict, metaObjectAttr); + // PYSIDE-813: The above assumption is not true in debug mode: + // PyDict_GetItem would touch PyThreadState_GET and the global error state. + // PyDict_GetItemWithError instead can work without GIL. + PyObject *pyBuilder = PyDict_GetItemWithError(dict, metaObjectAttr); #ifdef IS_PY3K return reinterpret_cast<MetaObjectBuilder *>(PyCapsule_GetPointer(pyBuilder, nullptr)); #else diff --git a/sources/pyside2/libpyside/signalmanager.h b/sources/pyside2/libpyside/signalmanager.h index 229ddb91d..fe077bd1a 100644 --- a/sources/pyside2/libpyside/signalmanager.h +++ b/sources/pyside2/libpyside/signalmanager.h @@ -43,6 +43,7 @@ #include "pysidemacros.h" #include <sbkpython.h> +#include <shibokenmacros.h> #include <QtCore/QMetaMethod> diff --git a/sources/shiboken2/libshiboken/pep384impl.cpp b/sources/shiboken2/libshiboken/pep384impl.cpp index 9939bb410..9f51034f3 100644 --- a/sources/shiboken2/libshiboken/pep384impl.cpp +++ b/sources/shiboken2/libshiboken/pep384impl.cpp @@ -518,12 +518,57 @@ static PyTypeObject *getFunctionType(void) return reinterpret_cast<PyTypeObject *>(PepRun_GetResult(prog)); } +#endif // Py_LIMITED_API || Python 2 + +/***************************************************************************** + * + * Support for dictobject.h + * + */ + +// PYSIDE-803, PYSIDE-813: We need that GIL-free version from Python 2.7.12 . +#if PY_VERSION_HEX < 0x03000000 + +/* Variant of PyDict_GetItem() that doesn't suppress exceptions. + This returns NULL *with* an exception set if an exception occurred. + It returns NULL *without* an exception set if the key wasn't present. +*/ +PyObject * +PyDict_GetItemWithError(PyObject *op, PyObject *key) +{ + long hash; + PyDictObject *mp = reinterpret_cast<PyDictObject *>(op); + PyDictEntry *ep; + if (!PyDict_Check(op)) { + PyErr_BadInternalCall(); + return nullptr; + } + if (!PyString_CheckExact(key) || + (hash = (reinterpret_cast<PyStringObject *>(key))->ob_shash) == -1) + { + hash = PyObject_Hash(key); + if (hash == -1) { + return nullptr; + } + } + + ep = (mp->ma_lookup)(mp, key, hash); + if (ep == nullptr) { + return nullptr; + } + return ep->me_value; +} + +#endif + /***************************************************************************** * * Extra support for signature.cpp * */ +#ifdef Py_LIMITED_API + PyTypeObject *PepStaticMethod_TypePtr = nullptr; static PyTypeObject * diff --git a/sources/shiboken2/libshiboken/pep384impl.h b/sources/shiboken2/libshiboken/pep384impl.h index 676e6c82d..e735095e8 100644 --- a/sources/shiboken2/libshiboken/pep384impl.h +++ b/sources/shiboken2/libshiboken/pep384impl.h @@ -279,6 +279,17 @@ LIBSHIBOKEN_API char *_PepUnicode_AsString(PyObject *); /***************************************************************************** * + * RESOLVED: dictobject.h + * + * PYSIDE-803, PYSIDE-813: We need PyDict_GetItemWithError in order to + * avoid the GIL. + */ +#if PY_VERSION_HEX < 0x03000000 +LIBSHIBOKEN_API PyObject *PyDict_GetItemWithError(PyObject *mp, PyObject *key); +#endif + +/***************************************************************************** + * * RESOLVED: methodobject.h * */ diff --git a/sources/shiboken2/libshiboken/pep384impl_doc.rst b/sources/shiboken2/libshiboken/pep384impl_doc.rst index ac4b053a7..d8ebdbe70 100644 --- a/sources/shiboken2/libshiboken/pep384impl_doc.rst +++ b/sources/shiboken2/libshiboken/pep384impl_doc.rst @@ -106,6 +106,16 @@ listobject.h function calls. +dictobject.h +------------ + +``PyDict_GetItem`` also exists in a ``PyDict_GetItemWithError`` version that does +not suppress errors. This suppression has the side effect of touching global +structures. This function exists in Python 2 only since Python 2.7.12 and has +a different name. We simply implemented the function. +Needed to avoid the GIL when accessing dictionaries. + + methodobject.h -------------- |