From 95abd1ab5447f320179a1104698888ac2ef3e6b8 Mon Sep 17 00:00:00 2001 From: John Loy Date: Mon, 3 Nov 2014 14:23:39 -0800 Subject: [PATCH 1/2] Hold the GIL while releasing shared_ptr_deleter's python handle. Python requires that the GIL be held while releasing objects, but requiring that all clients of boost::shared_ptr-held types do so explicitly is error-prone. In the case where the wrapped type is consumed in a library that you don't control, it might not even be possible to do correctly. Instead, have shared_ptr_deleter acquire the GIL on client's behalf at exactly the time when it is necessary. --- src/converter/builtin_converters.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/converter/builtin_converters.cpp b/src/converter/builtin_converters.cpp index 9900602b7..9bd021928 100644 --- a/src/converter/builtin_converters.cpp +++ b/src/converter/builtin_converters.cpp @@ -30,8 +30,30 @@ shared_ptr_deleter::shared_ptr_deleter(handle<> owner) shared_ptr_deleter::~shared_ptr_deleter() {} +namespace +{ + + class scoped_gil_state + { + public: + scoped_gil_state() + : m_gil_state(PyGILState_Ensure()) + {} + + ~scoped_gil_state() + { + PyGILState_Release(m_gil_state); + } + + private: + PyGILState_STATE m_gil_state; + }; + +} + void shared_ptr_deleter::operator()(void const*) { + scoped_gil_state gil; owner.reset(); } From 51d963f0dc6f28084b5a848c9486baedbf507b60 Mon Sep 17 00:00:00 2001 From: John Loy Date: Tue, 11 Nov 2014 13:57:19 -0800 Subject: [PATCH 2/2] Limit shared_ptr_deleter's GIL acquisition to clients with multithreading enabled & a "new enough" Python version. Only compile in this behavior for Python 2.4 & newer (because PyEval_ThreadsInitialized was added in 2.4) and only if Python was compiled with threading support (WITH_THREAD) so that Boost Python continues to work as-is for other users. Only acquire the GIL if threading support has been initialized at runtime to help avoid overhead in clients that don't require it. --- src/converter/builtin_converters.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/converter/builtin_converters.cpp b/src/converter/builtin_converters.cpp index 9bd021928..b34f633df 100644 --- a/src/converter/builtin_converters.cpp +++ b/src/converter/builtin_converters.cpp @@ -30,6 +30,7 @@ shared_ptr_deleter::shared_ptr_deleter(handle<> owner) shared_ptr_deleter::~shared_ptr_deleter() {} +#if PY_VERSION_HEX >= 0x02040000 && defined(WITH_THREAD) namespace { @@ -37,23 +38,32 @@ namespace { public: scoped_gil_state() - : m_gil_state(PyGILState_Ensure()) - {} + { + if ((m_py_threads_initialized = PyEval_ThreadsInitialized())) { + m_gil_state = PyGILState_Ensure(); + } + } ~scoped_gil_state() { - PyGILState_Release(m_gil_state); + if (m_py_threads_initialized) { + PyGILState_Release(m_gil_state); + } } private: PyGILState_STATE m_gil_state; + bool m_py_threads_initialized; }; } +#endif void shared_ptr_deleter::operator()(void const*) { +#if PY_VERSION_HEX >= 0x02040000 && defined(WITH_THREAD) scoped_gil_state gil; +#endif owner.reset(); }