Skip to content

Commit faca353

Browse files
committed
[225] Use PyGILState_* API for thread handling
1 parent a648780 commit faca353

1 file changed

Lines changed: 25 additions & 39 deletions

File tree

src/main.cpp

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -106,19 +106,12 @@ namespace
106106
static inline constexpr const char* const rule_engine_name = "python";
107107
using log_re = irods::experimental::log::rule_engine;
108108

109-
// Python thread states and other data from the interpreter
109+
// Data we hang onto for the interpreter
110110
namespace python_state
111111
{
112112
// Thread state object for main Python interpreter
113113
static PyThreadState* ts_main;
114114

115-
// Thread state object for current thread
116-
static thread_local PyThreadState* ts_thread;
117-
// Pre-initialize thread state object for current thread
118-
static thread_local PyThreadState* ts_thread_old;
119-
// Reference counter for nested python operations
120-
static thread_local uint64_t ts_thread_refct = 0;
121-
122115
#if PY_VERSION_HEX >= 0x03080000
123116
// List of default module search paths
124117
static std::vector<std::wstring> default_module_search_paths;
@@ -253,34 +246,27 @@ namespace
253246
return rei;
254247
}
255248

256-
// Helper struct for managing python thread state
257-
struct python_thread_state_scope
249+
// Helper class that acquires the GIL while in scope
250+
class python_gil_lock
258251
{
259-
python_thread_state_scope()
260-
{
261-
if (0 == python_state::ts_thread_refct) {
262-
python_state::ts_thread = PyThreadState_New(python_state::ts_main->interp);
263-
PyEval_RestoreThread(python_state::ts_thread);
264-
python_state::ts_thread_old = PyThreadState_Swap(python_state::ts_thread);
265-
}
266-
python_state::ts_thread_refct++;
267-
}
252+
public:
253+
python_gil_lock()
254+
: _previous_gil_state(PyGILState_Ensure())
255+
{ }
268256

269-
~python_thread_state_scope()
257+
~python_gil_lock()
270258
{
271-
if (1 == python_state::ts_thread_refct) {
272-
PyThreadState_Swap(python_state::ts_thread_old);
273-
PyThreadState_Clear(python_state::ts_thread);
274-
PyThreadState_DeleteCurrent();
275-
}
276-
python_state::ts_thread_refct--;
259+
PyGILState_Release(_previous_gil_state);
277260
}
278261

279-
python_thread_state_scope(const python_thread_state_scope&) = delete;
262+
python_gil_lock(const python_gil_lock&) = delete;
263+
264+
python_gil_lock& operator=(const python_gil_lock&) = delete;
280265

281-
python_thread_state_scope& operator=(const python_thread_state_scope&) = delete;
266+
private:
267+
PyGILState_STATE _previous_gil_state; // GIL state prior to calling PyGILState_Ensure
282268

283-
}; //struct python_thread_state_scope
269+
}; //class python_gil_lock
284270

285271
struct RuleCallWrapper
286272
{
@@ -552,7 +538,7 @@ static irods::error rule_exists(const irods::default_re_ctx&, const std::string&
552538
{
553539
_return = false;
554540
std::lock_guard<std::recursive_mutex> lock{python_mutex};
555-
python_thread_state_scope tstate;
541+
python_gil_lock gil_lock;
556542
try {
557543
// TODO Enable non core.py Python rulebases
558544
bp::object core_module = bp::import("core");
@@ -572,7 +558,7 @@ static irods::error rule_exists(const irods::default_re_ctx&, const std::string&
572558
return ERROR(RULE_ENGINE_ERROR, err_msg);
573559
}
574560
// NOTE: If adding more catch blocks, nest this try/catch in another try block
575-
// along with the lock and tstate definitions. They need to stay in-scope for extract_python_exception,
561+
// along with the lock and gil_lock definitions. They need to stay in-scope for extract_python_exception,
576562
// but should be out of scope for other exception handlers.
577563

578564
return SUCCESS();
@@ -582,8 +568,8 @@ static irods::error list_rules(const irods::default_re_ctx&, std::vector<std::st
582568
{
583569
try {
584570
std::lock_guard<std::recursive_mutex> lock{python_mutex};
585-
python_thread_state_scope tstate;
586-
// tstate (and therefore also lock) needs to stay in scope for extract_python_exception
571+
python_gil_lock gil_lock;
572+
// gil_lock (and therefore also lock) needs to stay in scope for extract_python_exception
587573
// hence the nested exception handling
588574
try {
589575
bp::object core_module = bp::import("core");
@@ -649,8 +635,8 @@ static irods::error exec_rule(const irods::default_re_ctx&,
649635
{
650636
try {
651637
std::lock_guard<std::recursive_mutex> lock{python_mutex};
652-
python_thread_state_scope tstate;
653-
// tstate (and therefore also lock) needs to stay in scope for extract_python_exception
638+
python_gil_lock gil_lock;
639+
// gil_lock (and therefore also lock) needs to stay in scope for extract_python_exception
654640
// hence the nested exception handling
655641
try {
656642
// TODO Enable non core.py Python rulebases
@@ -776,8 +762,8 @@ static irods::error exec_rule_text(const irods::default_re_ctx&,
776762

777763
try {
778764
std::lock_guard<std::recursive_mutex> lock{python_mutex};
779-
python_thread_state_scope tstate;
780-
// tstate (and therefore also lock) needs to stay in scope for extract_python_exception
765+
python_gil_lock gil_lock;
766+
// gil_lock (and therefore also lock) needs to stay in scope for extract_python_exception
781767
// hence the nested exception handling
782768
try {
783769
execCmdOut_t* myExecCmdOut = (execCmdOut_t*) malloc(sizeof(*myExecCmdOut));
@@ -930,8 +916,8 @@ static irods::error exec_rule_expression(irods::default_re_ctx&,
930916
{
931917
try {
932918
std::lock_guard<std::recursive_mutex> lock{python_mutex};
933-
python_thread_state_scope tstate;
934-
// tstate (and therefore also lock) needs to stay in scope for extract_python_exception
919+
python_gil_lock gil_lock;
920+
// gil_lock (and therefore also lock) needs to stay in scope for extract_python_exception
935921
// hence the nested exception handling
936922
try {
937923
bp::dict rule_vars_python;

0 commit comments

Comments
 (0)