Skip to content

Commit

Permalink
Split py_context into py_thread_context and py_ensure_gil.
Browse files Browse the repository at this point in the history
Slightly different handling of the GIL is required for newly-created C
threads vs functions that are either running in the main thread or from
a correctly-initialised C thread.
  • Loading branch information
iguanaonmystack committed Apr 25, 2020
1 parent 331c99a commit d1c86ff
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 13 deletions.
22 changes: 19 additions & 3 deletions src/helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
#include "helpers.h"
#include <Python.h>

class py_context {
/* entry points to threads should grab a py_thread_context for the duration of the thread */
class py_thread_context {
public:
py_context() { gstate = PyGILState_Ensure(); }
py_thread_context() { gstate = PyGILState_Ensure(); }

~py_context() {
~py_thread_context() {
PyGILState_Release(gstate);
if (PyGILState_Check() == 1)
pts = PyEval_SaveThread();
Expand All @@ -21,6 +22,21 @@ class py_context {
PyThreadState *pts;
};

/* anywhere needing to call python functions (that isn't using py_thread_context) should create a py_ensure_gil object */
class py_ensure_gil {
public:
py_ensure_gil() {
gstate = PyGILState_Ensure();
}

~py_ensure_gil() {
PyGILState_Release(gstate);
}

private:
PyGILState_STATE gstate;
};

// v8 to Python
PyObject *BuildPyArray(Napi::Env env, Napi::Value arg);
PyObject *BuildPyDict(Napi::Env env, Napi::Value arg);
Expand Down
14 changes: 9 additions & 5 deletions src/pynode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Napi::Value StartInterpreter(const Napi::CallbackInfo &info) {
.ThrowAsJavaScriptException();
}

/* Release the GIL. The other entry points back into Python re-acquire it */
PyEval_SaveThread();

return env.Null();
}

Expand Down Expand Up @@ -86,7 +89,7 @@ Napi::Value AppendSysPath(const Napi::CallbackInfo &info) {
pathName.c_str());

{
py_context ctx;
py_ensure_gil ctx;
PyRun_SimpleString(appendPathStr);
free(appendPathStr);
}
Expand All @@ -106,7 +109,7 @@ Napi::Value OpenFile(const Napi::CallbackInfo &info) {
std::string fileName = info[0].As<Napi::String>().ToString();

{
py_context ctx;
py_ensure_gil ctx;

PyObject *pName;
pName = PyUnicode_DecodeFSDefault(fileName.c_str());
Expand Down Expand Up @@ -136,7 +139,7 @@ Napi::Value ImportModule(const Napi::CallbackInfo &info) {
std::string moduleName = info[0].As<Napi::String>().ToString();

{
py_context ctx;
py_ensure_gil ctx;

PyObject *pName;
pName = PyUnicode_FromString(moduleName.c_str());
Expand Down Expand Up @@ -167,7 +170,7 @@ Napi::Value Eval(const Napi::CallbackInfo &info) {
std::string statement = info[0].As<Napi::String>().ToString();
int response;
{
py_context ctx;
py_ensure_gil ctx;

response = PyRun_SimpleString(statement.c_str());
}
Expand Down Expand Up @@ -197,7 +200,8 @@ Napi::Value Call(const Napi::CallbackInfo &info) {
PyObject *pFunc, *pArgs;

{
py_context ctx;
printf("getting gil\n");
py_ensure_gil ctx;

pFunc = PyObject_GetAttrString(pModule, functionName.c_str());
int callable = PyCallable_Check(pFunc);
Expand Down
6 changes: 3 additions & 3 deletions src/pywrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ PyNodeWrappedPythonObject::PyNodeWrappedPythonObject(const Napi::CallbackInfo &i
Napi::FunctionReference PyNodeWrappedPythonObject::constructor;

Napi::Value PyNodeWrappedPythonObject::GetAttr(const Napi::CallbackInfo &info){
py_context ctx;
py_ensure_gil ctx;
Napi::Env env = info.Env();
std::string attrname = info[0].As<Napi::String>().ToString();
PyObject * attr = PyObject_GetAttrString(this->_value, attrname.c_str());
Expand All @@ -48,7 +48,7 @@ Napi::Value PyNodeWrappedPythonObject::GetAttr(const Napi::CallbackInfo &info){
}

Napi::Value PyNodeWrappedPythonObject::Call(const Napi::CallbackInfo &info){
py_context ctx;
py_ensure_gil ctx;
Napi::Env env = info.Env();
int callable = PyCallable_Check(this->_value);
if (! callable) {
Expand All @@ -73,7 +73,7 @@ Napi::Value PyNodeWrappedPythonObject::Call(const Napi::CallbackInfo &info){
}

Napi::Value PyNodeWrappedPythonObject::Repr(const Napi::CallbackInfo &info){
py_context ctx;
py_ensure_gil ctx;
Napi::Env env = info.Env();
std::string attrname = info[0].As<Napi::String>().ToString();
PyObject * repr = PyObject_Repr(this->_value);
Expand Down
4 changes: 2 additions & 2 deletions src/worker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ PyNodeWorker::~PyNodeWorker(){};

void PyNodeWorker::Execute() {
{
py_context ctx;
py_thread_context ctx;

pValue = PyObject_CallObject(pFunc, pyArgs);
Py_DECREF(pyArgs);
Expand Down Expand Up @@ -47,7 +47,7 @@ void PyNodeWorker::Execute() {
}

void PyNodeWorker::OnOK() {
py_context ctx;
py_thread_context ctx;
Napi::HandleScope scope(Env());

std::vector<Napi::Value> result = {Env().Null(), Env().Null()};
Expand Down

0 comments on commit d1c86ff

Please sign in to comment.