From 7f463b313c6dee9d1b90a3aba631855035203e94 Mon Sep 17 00:00:00 2001 From: AntonyA Date: Fri, 23 Feb 2018 14:06:26 +0800 Subject: [PATCH] app_python3: implement script reloading Copy the method used in app_lua: the version of the script is incremented by RPC. In python_exec.c:apy_exec() check for a newer version and reload the script. Set a lock so the script reload only occurs at depth 0 (in the unlikely case that apy_exec() is called recursively). This is not thread-safe as we are using a process-wide lock: don't call back into apy_exec() from a Python extension that uses threads. --- src/modules/app_python3/app_python_mod.c | 220 +++++++++++------- src/modules/app_python3/app_python_mod.h | 2 +- src/modules/app_python3/apy_kemi.c | 47 +--- .../app_python3/doc/app_python3_admin.xml | 14 +- src/modules/app_python3/python_exec.c | 98 +++----- 5 files changed, 191 insertions(+), 190 deletions(-) diff --git a/src/modules/app_python3/app_python_mod.c b/src/modules/app_python3/app_python_mod.c index 837f8a6951e..ada2bab4744 100644 --- a/src/modules/app_python3/app_python_mod.c +++ b/src/modules/app_python3/app_python_mod.c @@ -51,7 +51,7 @@ static int mod_init(void); static int child_init(int rank); static void mod_destroy(void); -PyObject *_sr_apy_handler_obj; +PyObject *_sr_apy_handler_obj = NULL; char *dname = NULL, *bname = NULL; @@ -197,161 +197,215 @@ static void mod_destroy(void) #define PY_GIL_ENSURE gstate = PyGILState_Ensure(); #define PY_GIL_RELEASE PyGILState_Release(gstate); - -// #define PY_THREADSTATE_SWAP_IN PyThreadState_Swap(myThreadState); -// #define PY_THREADSTATE_SWAP_NULL PyThreadState_Swap(NULL); -#define PY_THREADSTATE_SWAP_IN -#define PY_THREADSTATE_SWAP_NULL - -int apy_load_script(void) +int apy_mod_init(PyObject* pModule) { - PyObject *sys_path, *pDir, *pModule, *pFunc, *pArgs; - PyThreadState *mainThreadState; - PyGILState_STATE gstate; - - if (ap_init_modules() != 0) { - return -1; - } - Py_Initialize(); - PyEval_InitThreads(); - myThreadState = PyThreadState_Get(); + /* + * pModule: managed by caller, no need to Py_DECREF + */ + PyObject *pFunc, *pArgs, *pHandler; + PyGILState_STATE gstate; PY_GIL_ENSURE - format_exc_obj = InitTracebackModule(); + pFunc = PyObject_GetAttrString(pModule, mod_init_fname.s); - if (format_exc_obj == NULL || !PyCallable_Check(format_exc_obj)) - { - Py_XDECREF(format_exc_obj); - PY_GIL_RELEASE - return -1; - } + /* pFunc is a new reference */ - sys_path = PySys_GetObject("path"); - /* PySys_GetObject doesn't pass reference! No need to DEREF */ - if (sys_path == NULL) { + if (pFunc == NULL) { if (!PyErr_Occurred()) PyErr_Format(PyExc_AttributeError, - "'module' object 'sys' has no attribute 'path'"); + "'module' object '%s' has no attribute '%s'", + bname, mod_init_fname.s); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); + Py_XDECREF(pFunc); PY_GIL_RELEASE return -1; } - pDir = PyUnicode_FromString(dname); - if (pDir == NULL) { + if (!PyCallable_Check(pFunc)) { if (!PyErr_Occurred()) PyErr_Format(PyExc_AttributeError, - "PyUnicode_FromString() has failed"); + "module object '%s' has is not callable attribute '%s'", + bname, mod_init_fname.s); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); + Py_XDECREF(pFunc); PY_GIL_RELEASE return -1; } - PyList_Insert(sys_path, 0, pDir); - Py_DECREF(pDir); - if (python_msgobj_init() != 0) { - if (!PyErr_Occurred()) - PyErr_SetString(PyExc_AttributeError, - "python_msgobj_init() has failed"); + pArgs = PyTuple_New(0); + if (pArgs == NULL) { python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); + Py_DECREF(pFunc); PY_GIL_RELEASE return -1; } - pModule = PyImport_ImportModule(bname); - if (pModule == NULL) { - PyErr_PrintEx(0); + pHandler = PyObject_CallObject(pFunc, pArgs); + + Py_XDECREF(pFunc); + Py_XDECREF(pArgs); + + if (pHandler == Py_None) { if (!PyErr_Occurred()) - PyErr_Format(PyExc_ImportError, "No module named '%s'", bname); + PyErr_Format(PyExc_TypeError, + "Function '%s' of module '%s' has returned None." + " Should be a class instance.", mod_init_fname.s, bname); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); PY_GIL_RELEASE - return -1; } - pFunc = PyObject_GetAttrString(pModule, mod_init_fname.s); - Py_DECREF(pModule); - - /* pFunc is a new reference */ - - if (pFunc == NULL) { - if (!PyErr_Occurred()) - PyErr_Format(PyExc_AttributeError, - "'module' object '%s' has no attribute '%s'", - bname, mod_init_fname.s); + if (PyErr_Occurred()) { python_handle_exception("mod_init"); + Py_XDECREF(_sr_apy_handler_obj); Py_DECREF(format_exc_obj); - Py_XDECREF(pFunc); PY_GIL_RELEASE return -1; } - if (!PyCallable_Check(pFunc)) { + if (pHandler == NULL) { + LM_ERR("PyObject_CallObject() returned NULL but no exception!\n"); if (!PyErr_Occurred()) - PyErr_Format(PyExc_AttributeError, - "module object '%s' has is not callable attribute '%s'", - bname, mod_init_fname.s); + PyErr_Format(PyExc_TypeError, + "Function '%s' of module '%s' has returned not returned" + " object. Should be a class instance.", + mod_init_fname.s, bname); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); - Py_XDECREF(pFunc); PY_GIL_RELEASE return -1; } + Py_XDECREF(_sr_apy_handler_obj); + _sr_apy_handler_obj = pHandler; + PY_GIL_RELEASE + return 0; +} - pArgs = PyTuple_New(0); - if (pArgs == NULL) { +/* + * reference to the module to allow reload + */ +static PyObject *_sr_apy_module; + +int apy_reload_script(void) +{ + PyGILState_STATE gstate; + + PY_GIL_ENSURE + PyObject *pModule = PyImport_ReloadModule(_sr_apy_module); + if (!pModule) { + PyErr_PrintEx(0); + if (!PyErr_Occurred()) + PyErr_Format(PyExc_ImportError, "Reload module '%s'", bname); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); - Py_DECREF(pFunc); PY_GIL_RELEASE + return -1; } + if (apy_mod_init(pModule)) { + LM_ERR("Error calling mod_init on reload\n"); + Py_DECREF(pModule); + PY_GIL_RELEASE + return -1; - _sr_apy_handler_obj = PyObject_CallObject(pFunc, pArgs); + } + Py_DECREF(_sr_apy_module); + _sr_apy_module = pModule; - Py_XDECREF(pFunc); - Py_XDECREF(pArgs); + if(apy_init_script(_apy_process_rank)<0) { + LM_ERR("failed to init script\n"); + return -1; + } + PY_GIL_RELEASE + return 0; +} + +int apy_load_script(void) +{ + PyObject *sys_path, *pDir, *pModule; + PyGILState_STATE gstate; + + if (ap_init_modules() != 0) { + return -1; + } + + Py_Initialize(); + PyEval_InitThreads(); + myThreadState = PyThreadState_Get(); + + PY_GIL_ENSURE + format_exc_obj = InitTracebackModule(); + + if (format_exc_obj == NULL || !PyCallable_Check(format_exc_obj)) + { + Py_XDECREF(format_exc_obj); + PY_GIL_RELEASE + return -1; + } + + sys_path = PySys_GetObject("path"); + /* PySys_GetObject doesn't pass reference! No need to DEREF */ + if (sys_path == NULL) { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_AttributeError, + "'module' object 'sys' has no attribute 'path'"); + python_handle_exception("mod_init"); + Py_DECREF(format_exc_obj); + PY_GIL_RELEASE + return -1; + } - if (_sr_apy_handler_obj == Py_None) { + pDir = PyUnicode_FromString(dname); + if (pDir == NULL) { if (!PyErr_Occurred()) - PyErr_Format(PyExc_TypeError, - "Function '%s' of module '%s' has returned None." - " Should be a class instance.", mod_init_fname.s, bname); + PyErr_Format(PyExc_AttributeError, + "PyUnicode_FromString() has failed"); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); PY_GIL_RELEASE return -1; } - if (PyErr_Occurred()) { + PyList_Insert(sys_path, 0, pDir); + Py_DECREF(pDir); + + if (python_msgobj_init() != 0) { + if (!PyErr_Occurred()) + PyErr_SetString(PyExc_AttributeError, + "python_msgobj_init() has failed"); python_handle_exception("mod_init"); - Py_XDECREF(_sr_apy_handler_obj); Py_DECREF(format_exc_obj); PY_GIL_RELEASE return -1; } - if (_sr_apy_handler_obj == NULL) { - LM_ERR("PyObject_CallObject() returned NULL but no exception!\n"); + pModule = PyImport_ImportModule(bname); + if (pModule == NULL) { + PyErr_PrintEx(0); if (!PyErr_Occurred()) - PyErr_Format(PyExc_TypeError, - "Function '%s' of module '%s' has returned not returned" - " object. Should be a class instance.", - mod_init_fname.s, bname); + PyErr_Format(PyExc_ImportError, "No module named '%s'", bname); python_handle_exception("mod_init"); Py_DECREF(format_exc_obj); PY_GIL_RELEASE + return -1; } + if (apy_mod_init(pModule) != 0) { + LM_ERR("Error calling mod_init\n"); + Py_DECREF(pModule); + PY_GIL_RELEASE + return -1; + + } + _sr_apy_module = pModule; - //myThreadState = PyThreadState_New(mainThreadState->interp); PY_GIL_RELEASE return 0; } @@ -365,7 +419,6 @@ int apy_init_script(int rank) PY_GIL_ENSURE - PY_THREADSTATE_SWAP_IN // get instance class name classname = get_instance_class_name(_sr_apy_handler_obj); @@ -376,7 +429,6 @@ int apy_init_script(int rank) "'module' instance has no class name"); python_handle_exception("child_init"); Py_DECREF(format_exc_obj); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -387,7 +439,6 @@ int apy_init_script(int rank) python_handle_exception("child_init"); Py_XDECREF(pFunc); Py_DECREF(format_exc_obj); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -400,7 +451,6 @@ int apy_init_script(int rank) python_handle_exception("child_init"); Py_DECREF(format_exc_obj); Py_XDECREF(pFunc); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -410,7 +460,6 @@ int apy_init_script(int rank) python_handle_exception("child_init"); Py_DECREF(format_exc_obj); Py_DECREF(pFunc); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -421,7 +470,6 @@ int apy_init_script(int rank) Py_DECREF(format_exc_obj); Py_DECREF(pArgs); Py_DECREF(pFunc); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -436,14 +484,12 @@ int apy_init_script(int rank) python_handle_exception("child_init"); Py_DECREF(format_exc_obj); Py_XDECREF(pResult); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } if (pResult == NULL) { LM_ERR("PyObject_CallObject() returned NULL but no exception!\n"); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } @@ -457,14 +503,12 @@ int apy_init_script(int rank) python_handle_exception("child_init"); Py_DECREF(format_exc_obj); Py_XDECREF(pResult); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return -1; } rval = PyLong_AsLong(pResult); Py_DECREF(pResult); - PY_THREADSTATE_SWAP_NULL PY_GIL_RELEASE return rval; diff --git a/src/modules/app_python3/app_python_mod.h b/src/modules/app_python3/app_python_mod.h index 89016150163..0c108e2d3fb 100644 --- a/src/modules/app_python3/app_python_mod.h +++ b/src/modules/app_python3/app_python_mod.h @@ -27,5 +27,5 @@ extern PyObject *_sr_apy_handler_obj; extern PyObject *format_exc_obj; extern PyThreadState *myThreadState; - +int apy_reload_script(void); #endif diff --git a/src/modules/app_python3/apy_kemi.c b/src/modules/app_python3/apy_kemi.c index bce5e2aeba3..de342c2058d 100644 --- a/src/modules/app_python3/apy_kemi.c +++ b/src/modules/app_python3/apy_kemi.c @@ -28,6 +28,7 @@ #include "../../core/route.h" #include "../../core/fmsg.h" #include "../../core/kemi.h" +#include "../../core/locking.h" #include "../../core/pvar.h" #include "../../core/mem/pkg.h" #include "../../core/mem/shm.h" @@ -39,34 +40,13 @@ #include "apy_kemi_export.h" #include "apy_kemi.h" -static int *_sr_python_reload_version = NULL; -static int _sr_python_local_version = 0; +int *_sr_python_reload_version = NULL; +int _sr_python_local_version = 0; +gen_lock_t* _sr_python_reload_lock; extern str _sr_python_load_file; extern int _apy_process_rank; -/** - * - */ - -int apy_reload_script(void) -{ - if(_sr_python_reload_version == NULL) { - return 0; - } - if(*_sr_python_reload_version == _sr_python_local_version) { - return 0; - } - if(apy_load_script()<0) { - LM_ERR("failed to load script file\n"); - return -1; - } - if(apy_init_script(_apy_process_rank)<0) { - LM_ERR("failed to init script\n"); - return -1; - } - _sr_python_local_version = *_sr_python_reload_version; - return 0; -} +int apy_reload_script(void); /** * */ @@ -1171,7 +1151,8 @@ int apy_sr_init_mod(void) } *_sr_python_reload_version = 0; } - + _sr_python_reload_lock = lock_alloc(); + lock_init(_sr_python_reload_lock); return 0; } @@ -1197,16 +1178,12 @@ static void app_python_rpc_reload(rpc_t* rpc, void* ctx) return; } - v = *_sr_python_reload_version; - LM_INFO("marking for reload js script file: %.*s (%d => %d)\n", - _sr_python_load_file.len, _sr_python_load_file.s, - _sr_python_local_version, v); + _sr_python_local_version = v = *_sr_python_reload_version; *_sr_python_reload_version += 1; - - if(apy_reload_script()<0) { - rpc->fault(ctx, 500, "Reload failed"); - return; - } + LM_INFO("marking for reload Python script file: %.*s (%d => %d)\n", + _sr_python_load_file.len, _sr_python_load_file.s, + v, + *_sr_python_reload_version); if (rpc->add(ctx, "{", &vh) < 0) { rpc->fault(ctx, 500, "Server error"); diff --git a/src/modules/app_python3/doc/app_python3_admin.xml b/src/modules/app_python3/doc/app_python3_admin.xml index 943a0065528..1ba487a6037 100644 --- a/src/modules/app_python3/doc/app_python3_admin.xml +++ b/src/modules/app_python3/doc/app_python3_admin.xml @@ -191,13 +191,19 @@ python_exec("my_python_function", "$rU"); app_python.reload - IMPORTANT: not functional yet (can crash a running instance, - use it only for testing). + IMPORTANT: this is not thread-safe. In your Python script + do not use C extensions with threads that call into + apy_exec(). Marks the need to reload the Python script. - The actual reload is done by every working process when the next - call to KEMI config is executed. + The actual reload is done in each worker when it next invokes a Python method. + The module uses a worker process lock to prevent recursive reloads. + + + This function only reloads the user script and creates a new script object. + It does not reinitialize the interpreter. + E.g., references in the old module remain if not redefined by the new version. Name: app_python.reload diff --git a/src/modules/app_python3/python_exec.c b/src/modules/app_python3/python_exec.c index ff471735ab0..74cb13a6931 100644 --- a/src/modules/app_python3/python_exec.c +++ b/src/modules/app_python3/python_exec.c @@ -30,6 +30,7 @@ #include "../../core/config.h" #include "../../core/mod_fix.h" #include "../../core/parser/parse_uri.h" +#include "../../core/locking.h" #include "python_exec.h" #include "app_python_mod.h" @@ -46,40 +47,44 @@ sr_apy_env_t *sr_apy_env_get() return &_sr_apy_env; } -static int _sr_apy_exec_pid = 0; - #define PY_GIL_ENSURE gstate = PyGILState_Ensure(); #define PY_GIL_RELEASE PyGILState_Release(gstate); +#define LOCK_RELEASE if(locked) lock_release(_sr_python_reload_lock); -// #define PY_THREADSTATE_SWAP_IN PyThreadState_Swap(myThreadState); -// #define PY_THREADSTATE_SWAP_NULL PyThreadState_Swap(NULL); -#define PY_THREADSTATE_SWAP_IN -#define PY_THREADSTATE_SWAP_NULL - -/** - * +/* + * copy the logic from app_lua/app_lua_api.c: + * reload script if version has changed and we are depth 0 + * initialized in apy_kemi.c */ +extern gen_lock_t* _sr_python_reload_lock; +extern int *_sr_python_reload_version; +extern int _sr_python_local_version; + int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) { PyObject *pFunc, *pArgs, *pValue, *pResult; PyObject *pmsg; - int rval; + int rval = -1; sip_msg_t *bmsg; - int mpid; - int locked = 0; PyGILState_STATE gstate; - - bmsg = _sr_apy_env.msg; - _sr_apy_env.msg = _msg; - mpid = getpid(); + int locked = 0; - if(_sr_apy_exec_pid!=mpid) { - PY_GIL_ENSURE - _sr_apy_exec_pid = mpid; - PY_THREADSTATE_SWAP_IN + if (lock_try(_sr_python_reload_lock) == 0) { + if(_sr_python_reload_version && *_sr_python_reload_version != _sr_python_local_version) { + LM_INFO("Reloading script %d->%d\n", _sr_python_local_version, *_sr_python_reload_version); + if (apy_reload_script()) { + LM_ERR("Error reloading script\n"); + } else { + _sr_python_local_version = *_sr_python_reload_version; + } + } locked = 1; } + 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)) { if(emode==1) { @@ -88,16 +93,12 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) LM_DBG("%s not found or is not callable\n", fname); } Py_XDECREF(pFunc); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; if(emode==1) { - return -1; + goto err; } else { - return 1; + rval = 1; + goto err; } } @@ -105,13 +106,8 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) if (pmsg == NULL) { LM_ERR("can't create MSGtype instance\n"); Py_DECREF(pFunc); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; - return -1; + goto err; } pArgs = PyTuple_New(fparam == NULL ? 1 : 2); @@ -120,13 +116,8 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) msg_invalidate(pmsg); Py_DECREF(pmsg); Py_DECREF(pFunc); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; - return -1; + goto err; } PyTuple_SetItem(pArgs, 0, pmsg); /* Tuple steals pmsg */ @@ -138,13 +129,8 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) msg_invalidate(pmsg); Py_DECREF(pArgs); Py_DECREF(pFunc); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; - return -1; + goto err; } PyTuple_SetItem(pArgs, 1, pValue); /* Tuple steals pValue */ @@ -157,34 +143,22 @@ int apy_exec(sip_msg_t *_msg, char *fname, char *fparam, int emode) if (PyErr_Occurred()) { Py_XDECREF(pResult); python_handle_exception("python_exec2"); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; - return -1; + goto err; } if (pResult == NULL) { LM_ERR("PyObject_CallObject() returned NULL\n"); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; - return -1; + goto err; } rval = PyLong_AsLong(pResult); Py_DECREF(pResult); - if(locked) { - _sr_apy_exec_pid = 0; - PY_THREADSTATE_SWAP_NULL - PY_GIL_RELEASE - } _sr_apy_env.msg = bmsg; + err: + PY_GIL_RELEASE + LOCK_RELEASE return rval; }