瀏覽代碼

app_python3: refactor GIL and thread state handling

* KEMI Python scripts that use Python threading for background jobs
  would observe that the Python threads are not running

  This is due to not releasing the GIL when returning to the Kamailio
  event loop from KEMI calls

* The module was using PyGILState_Ensure / PyGILState_Release to
  ensure thread-correctness

  It turns out that in this case these are the wrong functions to use

  The main thread is already correct due to Py_Initialize();
  the PyGILState_xxx functions are redundant

  To release the GIL so Python threads run we use the macros
  Py_BLOCK_THREADS/Py_UNBLOCK_THREADS

Note: the PyGILState_XXX functions are for C-threads created
by Python unaware libraries
S-P Chan 11 月之前
父節點
當前提交
0ffe157bc1
共有 3 個文件被更改,包括 61 次插入38 次删除
  1. 52 29
      src/modules/app_python3/app_python3_mod.c
  2. 0 2
      src/modules/app_python3/apy_kemi.h
  3. 9 7
      src/modules/app_python3/python_exec.c

+ 52 - 29
src/modules/app_python3/app_python3_mod.c

@@ -34,6 +34,8 @@
 #include "app_python3_mod.h"
 
 #include "apy_kemi.h"
+static int apy_load_script(void);
+static int apy_init_script(int rank);
 
 MODULE_VERSION
 
@@ -53,8 +55,6 @@ char *dname = NULL, *bname = NULL;
 
 int _apy_process_rank = 0;
 
-PyThreadState *myThreadState;
-
 /* clang-format off */
 /** module parameters */
 static param_export_t params[] = {
@@ -175,32 +175,46 @@ static int mod_init(void)
 /**
  *
  */
+__thread PyThreadState *_save;
+
 static int child_init(int rank)
 {
+	int ret = -1;
 	if(rank == PROC_INIT) {
 		/*
 		 * this is called before any process is forked
 		 * so the Python internal state handler
 		 * should be called now.
 		 */
+		/* clang-format off */
 #if PY_VERSION_HEX >= 0x03070000
+		Py_BLOCK_THREADS
 		PyOS_BeforeFork();
+		Py_UNBLOCK_THREADS
 #endif
 		return 0;
+		/* clang-format on */
 	}
 	if(rank == PROC_POSTCHILDINIT) {
 		/*
 		 * this is called after forking of all child
 		 * processes
 		 */
+		/* clang-format off */
 #if PY_VERSION_HEX >= 0x03070000
+		Py_BLOCK_THREADS
 		PyOS_AfterFork_Parent();
+		Py_UNBLOCK_THREADS
 #endif
 		return 0;
+		/* clang-format on */
 	}
 	_apy_process_rank = rank;
-
-	if(!_ksr_is_main) {
+	/* clang-format off */
+	Py_BLOCK_THREADS
+	if(!_ksr_is_main)
+	/* clang-format on */
+	{
 #if PY_VERSION_HEX >= 0x03070000
 		PyOS_AfterFork_Child();
 #else
@@ -208,9 +222,16 @@ static int child_init(int rank)
 #endif
 	}
 	if(cfg_child_init()) {
-		return -1;
+		ret = -1;
+		goto finish;
 	}
-	return apy_init_script(rank);
+	ret = apy_init_script(rank);
+
+finish:
+	/* clang-format off */
+	Py_UNBLOCK_THREADS
+	return ret;
+	/* clang-format on */
 }
 
 /**
@@ -224,20 +245,18 @@ static void mod_destroy(void)
 		free(bname); // bname was strdup'ed
 }
 
-
-#define PY_GIL_ENSURE gstate = PyGILState_Ensure()
-#define PY_GIL_RELEASE PyGILState_Release(gstate)
-int apy_mod_init(PyObject *pModule)
+/* Python module utility function
+ * - must be called with GIL held
+ */
+static int apy_mod_init(PyObject *pModule)
 {
 
 	/*
 	 * pModule: managed by caller, no need to Py_DECREF
 	 */
 	PyObject *pFunc, *pArgs, *pHandler;
-	PyGILState_STATE gstate;
 	int rval = -1;
 
-	PY_GIL_ENSURE;
 	pFunc = PyObject_GetAttrString(pModule, mod_init_fname.s);
 
 	/* pFunc is a new reference */
@@ -311,7 +330,6 @@ int apy_mod_init(PyObject *pModule)
 	_sr_apy_handler_obj = pHandler;
 	rval = 0;
 err:
-	PY_GIL_RELEASE;
 	return rval;
 }
 
@@ -323,11 +341,12 @@ static PyObject *_sr_apy_module;
 
 int apy_reload_script(void)
 {
-	PyGILState_STATE gstate;
 	int rval = -1;
 
-	PY_GIL_ENSURE;
+	/* clang-format off */
+	Py_BLOCK_THREADS
 	PyObject *pModule = PyImport_ReloadModule(_sr_apy_module);
+	/* clang-format on */
 	if(!pModule) {
 		if(!PyErr_Occurred())
 			PyErr_Format(PyExc_ImportError, "Reload module '%s'", bname);
@@ -349,16 +368,22 @@ int apy_reload_script(void)
 	}
 	rval = 0;
 err:
-	PY_GIL_RELEASE;
+	/* clang-format off */
+	Py_UNBLOCK_THREADS
 	return rval;
+	/* clang-format on */
 }
 
 #define INTERNAL_VERSION "1003\n"
 
-int apy_load_script(void)
+/* Entry point for the embedded interpreter
+ * - releases the GIL so Python threading
+ *   will run
+ */
+
+static int apy_load_script(void)
 {
 	PyObject *sys_path, *pDir, *pModule;
-	PyGILState_STATE gstate;
 	int rc, rval = -1;
 
 	if(sr_apy_init_ksr() != 0) {
@@ -366,12 +391,10 @@ int apy_load_script(void)
 	}
 
 	Py_Initialize();
+
 #if PY_VERSION_HEX < 0x03070000
 	PyEval_InitThreads();
 #endif
-	myThreadState = PyThreadState_Get();
-
-	PY_GIL_ENSURE;
 
 	// Py3 does not create a package-like hierarchy of modules
 	// make legacy modules importable using Py2 syntax
@@ -439,11 +462,17 @@ int apy_load_script(void)
 
 	rval = 0;
 err:
-	PY_GIL_RELEASE;
+	/* clang-format off */
+	Py_UNBLOCK_THREADS
 	return rval;
+	/* clang-format on */
 }
 
-int apy_init_script(int rank)
+/*
+ * Python script utility function
+ * - must be called with GIL held
+ */
+static int apy_init_script(int rank)
 {
 	PyObject *pFunc, *pArgs, *pValue, *pResult;
 	int rval = -1;
@@ -452,11 +481,6 @@ int apy_init_script(int rank)
 #else
 	char *classname;
 #endif
-	PyGILState_STATE gstate;
-
-
-	PY_GIL_ENSURE;
-
 	// get instance class name
 	classname = get_instance_class_name(_sr_apy_handler_obj);
 	if(classname == NULL) {
@@ -537,7 +561,6 @@ int apy_init_script(int rank)
 	rval = PyLong_AsLong(pResult);
 	Py_DECREF(pResult);
 err:
-	PY_GIL_RELEASE;
 	return rval;
 }
 /**

+ 0 - 2
src/modules/app_python3/apy_kemi.h

@@ -34,7 +34,5 @@ PyObject *sr_apy_kemi_exec_func(PyObject *self, PyObject *args, int idx);
 
 int apy_sr_init_mod(void);
 int app_python_init_rpc(void);
-int apy_load_script(void);
-int apy_init_script(int rank);
 
 #endif

+ 9 - 7
src/modules/app_python3/python_exec.c

@@ -15,10 +15,10 @@
  *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
- *
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
+
 #include <Python.h>
 
 #include "../../core/mem/mem.h"
@@ -47,8 +47,8 @@ sr_apy_env_t *sr_apy_env_get()
 	return &_sr_apy_env;
 }
 
-#define PY_GIL_ENSURE gstate = PyGILState_Ensure()
-#define PY_GIL_RELEASE PyGILState_Release(gstate)
+extern __thread PyThreadState *_save;
+
 #define LOCK_RELEASE \
 	if(locked)       \
 	lock_release(_sr_python_reload_lock)
@@ -68,9 +68,9 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode)
 	PyObject *pmsg;
 	int rval = -1;
 	sip_msg_t *bmsg;
-	PyGILState_STATE gstate;
 	int locked = 0;
 
+	Py_BLOCK_THREADS
 	/* clear error state */
 	PyErr_Clear();
 
@@ -90,7 +90,6 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode)
 
 	bmsg = _sr_apy_env.msg;
 	_sr_apy_env.msg = _msg;
-	PY_GIL_ENSURE;
 
 	pFunc = PyObject_GetAttrString(_sr_apy_handler_obj, fname);
 	if(pFunc == NULL || !PyCallable_Check(pFunc)) {
@@ -168,9 +167,12 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode)
 err:
 	/* clear error state */
 	PyErr_Clear();
-	PY_GIL_RELEASE;
 	LOCK_RELEASE;
+	/* clang-format off */
+	Py_UNBLOCK_THREADS
+
 	return rval;
+	/* clang-format on */
 }
 
 /**