1 files changed, 112 insertions, 99 deletions

diff --git a/pcilib/py.c b/pcilib/py.c
index 94d7ed8..03e9d8d 100644
--- a/pcilib/py.c
+++ b/pcilib/py.c
@@ -3,10 +3,9 @@
 #ifdef HAVE_PYTHON
 # include <Python.h>
 
-#if PY_MAJOR_VERSION >= 3
-#include <pthread.h>
-#endif /* PY_MAJOR_VERSION >= 3 */
-
+# if PY_MAJOR_VERSION >= 3
+#  include <pthread.h>
+# endif /* PY_MAJOR_VERSION >= 3 */
 #endif /* HAVE_PYTHON */
 
 #include <stdio.h>
@@ -22,7 +21,7 @@
 #include "error.h"
 
 #ifdef HAVE_PYTHON
-#define PCILIB_PYTHON_WRAPPER "pcipywrap"
+# define PCILIB_PYTHON_WRAPPER "pcipywrap"
 
 typedef struct pcilib_script_s pcilib_script_t;
 
@@ -32,14 +31,6 @@ struct pcilib_script_s {
     UT_hash_handle hh;			/**< hash */
 };
 
-#if PY_MAJOR_VERSION >= 3
-typedef struct pcilib_py_s_thread_control {
-   pthread_t pth;
-   pthread_cond_t cond_finished;
-   pthread_mutex_t cond_finished_lock;
-} pcilib_py_s_thread_control; 
-#endif /* PY_MAJOR_VERSION < 3 */
-
 struct pcilib_py_s {
     int finalyze; 			/**< Indicates, that we are initialized from wrapper and should not destroy Python resources in destructor */
     PyObject *main_module;		/**< Main interpreter */
@@ -47,9 +38,11 @@ struct pcilib_py_s {
     PyObject *pcilib_pywrap;		/**< pcilib wrapper module */
     pcilib_script_t *script_hash;	/**< Hash with loaded scripts */
     
-#if PY_MAJOR_VERSION >= 3
-    pcilib_py_s_thread_control *thr_ctl; /**< Controller for Python main loop thread for Python 3 */
-#endif /* PY_MAJOR_VERSION < 3 */
+# if PY_MAJOR_VERSION >= 3
+    pthread_t pth;
+    pthread_cond_t cond;
+    pthread_mutex_t lock;
+# endif /* PY_MAJOR_VERSION > 3 */
 };
 #endif /* HAVE_PYTHON */
 
@@ -72,7 +65,7 @@ void pcilib_log_python_error(const char *file, int line, pcilib_log_flags_t flag
 	PyErr_NormalizeException(&pytype, &pyval, &pytraceback);
 	if (pyval) pystr = PyObject_Str(pyval);
 
-#if PY_MAJOR_VERSION >= 3
+# if PY_MAJOR_VERSION >= 3
 	if (pytype) {
 	    if (PyUnicode_Check(pytype))
 		type = PyUnicode_AsUTF8(pytype);
@@ -82,7 +75,7 @@ void pcilib_log_python_error(const char *file, int line, pcilib_log_flags_t flag
 	if (pystr) {
 	    val = PyUnicode_AsUTF8(pystr);
 	}
-#else /* PY_MAJOR_VERSION >= 3 */
+# else /* PY_MAJOR_VERSION >= 3 */
 	if (pytype) {
 	    if (PyString_Check(pytype))
 		type = PyString_AsString(pytype);
@@ -92,7 +85,7 @@ void pcilib_log_python_error(const char *file, int line, pcilib_log_flags_t flag
 	if (pystr) {
 	    val = PyString_AsString(pystr);
 	}
-#endif /*PY_MAJOR_VERSION >= 3*/
+# endif /*PY_MAJOR_VERSION >= 3*/
     }
     PyGILState_Release(gstate);
 #endif /* HAVE_PYTHON */
@@ -133,26 +126,47 @@ void pcilib_log_python_error(const char *file, int line, pcilib_log_flags_t flag
 }
 
 #ifdef HAVE_PYTHON
-#if PY_MAJOR_VERSION >= 3
-void *pcilib_py_run_side_thread(void *arg) {
-   pcilib_t *ctx = (pcilib_t*)(arg);
-   //Initializing python
-   Py_Initialize();
-   PyEval_InitThreads();
-   PyEval_ReleaseLock();
+# if PY_MAJOR_VERSION >= 3
+/**
+ * Python3 specially treats the main thread intializing Python. It crashes if
+ * the Lock is released and any Python code is executed under the GIL compaling
+ * that GIL is not locked. Python3 assumes that the main thread most of the time
+ * holds the Lock, only shortly giving it away to other threads and re-obtaining 
+ * it hereafter. This is not possible to do with GILs, but instead (probably)
+ * PyEval_{Save,Restore}Thread() should be used. On other hand, the other threads
+ * are working fine with GILs. This makes things complicated as we need to know
+ * if we are running in main thread or not.
+ * To simplify matters, during initalization we start a new thread which will
+ * performa actual initialization of Python and, hence, act as main thread.
+ * We only intialize here. No python code is executed afterwards. So we don't
+ * need to care about special locking mechanisms in main thread. Instead all
+ * our user threads can use GILs normally. 
+ * See more details here:
+ * http://stackoverflow.com/questions/24499393/cpython-locking-the-gil-in-the-main-thread
+ * http://stackoverflow.com/questions/15470367/pyeval-initthreads-in-python-3-how-when-to-call-it-the-saga-continues-ad-naus
+ */
+static void *pcilib_py_run_init_thread(void *arg) {
+    pcilib_py_t *py = (pcilib_py_t*)(arg);
+
+    Py_Initialize();
+    PyEval_InitThreads();
+    PyEval_ReleaseLock();
+
+	// Ensure that main thread waiting for our signal
+    pthread_lock(&(py->lock));
    
-   //send initialization finish signal
-   pthread_cond_signal(&(ctx->py->thr_ctl->cond_finished));
-   pthread_mutex_unlock(&(ctx->py->thr_ctl->cond_finished_lock));
-   pthread_mutex_destroy(&(ctx->py->thr_ctl->cond_finished_lock));
-
-   //wait untill finish signal
-   pthread_mutex_lock(&(ctx->py->thr_ctl->cond_finished_lock));
-   pthread_cond_wait(&(ctx->py->thr_ctl->cond_finished), 
-		     &(ctx->py->thr_ctl->cond_finished_lock));
-   return NULL;
+	// Inform the parent thread that initialization is finished
+    pthread_cond_signal(&(py->cond));
+
+	// Wait untill cleanup is requested
+    pthread_cond_wait(&(py->cond), &(py->lock));
+    pthread_unlock(&(py->lock)));
+    
+    Py_Finalize();
+    
+    return NULL;
 }
-#endif /* PY_MAJOR_VERSION < 3 */
+# endif /* PY_MAJOR_VERSION < 3 */
 #endif /* HAVE_PYTHON */
 
 int pcilib_init_py(pcilib_t *ctx) {
@@ -161,28 +175,42 @@ int pcilib_init_py(pcilib_t *ctx) {
     if (!ctx->py) return PCILIB_ERROR_MEMORY;
     memset(ctx->py, 0, sizeof(pcilib_py_t));
 
-    if(!Py_IsInitialized()) {
-#if PY_MAJOR_VERSION >= 3
-      //create thread controller
-      ctx->py->thr_ctl = malloc(sizeof(pcilib_py_s_thread_control));
-      if(!ctx->py->thr_ctl) return PCILIB_ERROR_MEMORY;
-      memset(ctx->py->thr_ctl, 0, sizeof(pcilib_py_s_thread_control));
-      
-      //create side thread with python main loop
-      pthread_create(&(ctx->py->thr_ctl->pth), NULL, pcilib_py_run_side_thread, ctx);
-      pthread_mutex_lock(&(ctx->py->thr_ctl->cond_finished_lock));
-      
-      //wait until Python initializes
-      pthread_cond_wait(&(ctx->py->thr_ctl->cond_finished),
-			&(ctx->py->thr_ctl->cond_finished_lock));
-			
-#else /* PY_MAJOR_VERSION < 3 */
-      Py_Initialize();
-      // Since python is being initializing from c programm, it needs to initialize threads to work properly with c threads
-      PyEval_InitThreads();
-      PyEval_ReleaseLock();
-#endif /* PY_MAJOR_VERSION < 3 */
-      ctx->py->finalyze = 1;
+    if (!Py_IsInitialized()) {
+# if PY_MAJOR_VERSION < 3
+        Py_Initialize();
+    	    // Since python is being initializing from c programm, it needs to initialize threads to work properly with c threads
+        PyEval_InitThreads();
+        PyEval_ReleaseLock();
+# else /* PY_MAJOR_VERSION < 3 */
+	err = pthread_mutex_init(&(ctx->py.lock));
+	if (err) return PCILIB_ERROR_FAILED;
+
+	err = pthread_cond_init(&(ctx->py.cond));
+	if (err) {
+	    pthread_mutex_destroy(&(ctx->py.lock));
+	    return PCILIB_ERROR_FAILED;
+	}
+
+	err = pthread_mutex_lock(&(ctx->py.lock));
+	if (err) {
+	    pthread_cond_destroy(&(ctx->py.lock));
+	    pthread_mutex_destroy(&(ctx->py.lock));
+	    return PCILIB_ERROR_FAILED;
+	}
+
+	    // Create initalizer thread and wait until it releases the Lock
+        err = pthread_create(&(ctx->py.pth), NULL, pcilib_py_run_init_thread, &(ctx->py));
+	if (err) {
+	    pthread_mutex_unlock(&(ctx->py.lock));
+	    pthread_cond_destroy(&(ctx->py.cond));
+	    pthread_mutex_destroy(&(ctx->py.lock));
+	    return PCILIB_ERROR_FAILED;
+	}
+
+    	    // Wait until initialized and keep the lock afterwards until free executed
+	pthread_cond_wait(&(ctx->py.cond), (ctx->py.lock));
+# endif /* PY_MAJOR_VERSION < 3 */
+	ctx->py->finalyze = 1;
     }
 
 	
@@ -190,22 +218,22 @@ int pcilib_init_py(pcilib_t *ctx) {
 
     ctx->py->main_module = PyImport_AddModule("__parser__");
     if (!ctx->py->main_module) {
-	pcilib_python_warning("Error importing python parser");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Error importing python parser");
         return PCILIB_ERROR_FAILED;
     }
 
     ctx->py->global_dict = PyModule_GetDict(ctx->py->main_module);
     if (!ctx->py->global_dict) {
-	pcilib_python_warning("Error locating global python dictionary");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Error locating global python dictionary");
         return PCILIB_ERROR_FAILED;
     }
 
     PyObject *pywrap = PyImport_ImportModule(PCILIB_PYTHON_WRAPPER);
     if (!pywrap) {
-	pcilib_python_warning("Error importing pcilib python wrapper");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Error importing pcilib python wrapper");
 	return PCILIB_ERROR_FAILED;
     }
 	
@@ -216,8 +244,8 @@ int pcilib_init_py(pcilib_t *ctx) {
     Py_XDECREF(mod_name);
 	
     if (!ctx->py->pcilib_pywrap) {
-	pcilib_python_warning("Error initializing python wrapper");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Error initializing python wrapper");
         return PCILIB_ERROR_FAILED;
     }
     
@@ -253,15 +281,15 @@ int pcilib_py_add_script_dir(pcilib_t *ctx, const char *dir) {
 
     pypath = PySys_GetObject("path");
     if (!pypath) {
-	pcilib_python_warning("Can't get python path");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Can't get python path");
 	return PCILIB_ERROR_FAILED;
     }
 
     pynewdir = PyUnicode_FromString(script_dir);
     if (!pynewdir) {
-	pcilib_python_warning("Can't create python string");
 	PyGILState_Release(gstate);
+	pcilib_python_warning("Can't create python string");
 	return PCILIB_ERROR_MEMORY;
     }
     
@@ -290,13 +318,12 @@ int pcilib_py_add_script_dir(pcilib_t *ctx, const char *dir) {
     if (pyret) Py_DECREF(pyret);
     Py_DECREF(pynewdir);
 
+    PyGILState_Release(gstate);
+
     if (err) {
 	pcilib_python_warning("Can't add directory (%s) to python path", script_dir);
-	PyGILState_Release(gstate);
 	return err;
     }
-    
-    PyGILState_Release(gstate);
 #endif /* HAVE_PYTHON */
 
     return 0;
@@ -305,14 +332,13 @@ int pcilib_py_add_script_dir(pcilib_t *ctx, const char *dir) {
 void pcilib_free_py(pcilib_t *ctx) {
 #ifdef HAVE_PYTHON
     int finalyze = 0;
-    PyGILState_STATE gstate;
     
-#if PY_MAJOR_VERSION >= 3
-    pcilib_py_s_thread_control *thr_ctl = ctx->py->thr_ctl;
-#endif /* PY_MAJOR_VERSION < 3 */
-   
     if (ctx->py) {		
+	PyGILState_STATE gstate;
+
 	if (ctx->py->finalyze) finalyze = 1;
+	
+	gstate = PyGILState_Ensure();
 
 	if (ctx->py->script_hash) {
 	    pcilib_script_t *script, *script_tmp;
@@ -325,12 +351,11 @@ void pcilib_free_py(pcilib_t *ctx) {
 	    ctx->py->script_hash = NULL;
 	}
 
-	if (ctx->py->pcilib_pywrap) {
-	    gstate = PyGILState_Ensure();
+	if (ctx->py->pcilib_pywrap)
 	    Py_DECREF(ctx->py->pcilib_pywrap);
-	    PyGILState_Release(gstate);
-	 }
-	    
+
+	PyGILState_Release(gstate);
+    
 	
         free(ctx->py);
         ctx->py = NULL;
@@ -339,26 +364,16 @@ void pcilib_free_py(pcilib_t *ctx) {
     
     if (finalyze) {
 #if PY_MAJOR_VERSION >= 3
-
-       //stop python side thread
-       pthread_cond_signal(&(thr_ctl->cond_finished));
-       pthread_mutex_unlock(&(thr_ctl->cond_finished_lock));
-       pthread_join(thr_ctl->pth, NULL);
-       
-       //free python
-       //must be finalized in main thread to correctly stop python threading
-       PyGILState_Ensure();
-       Py_Finalize();
-       
-       //destroy thread controllers
-       pthread_mutex_destroy(&(thr_ctl->cond_finished_lock));
-       pthread_cond_destroy(&(thr_ctl->cond_finished));
-       free(thr_ctl);
+          // singal python init thread to stop and wait it to finish
+       pthread_cond_signal(&(ctx->py.cond));
+       pthread_mutex_unlock(&(ctx->py.lock));
+       pthread_join(ctx->py.pth, NULL);
        
+          // destroy synchronization primitives
+       pthread_cond_destroy(&(ctx->py.cond));
+       pthread_mutex_destroy(&(ctx->py.lock));
 #else /* PY_MAJOR_VERSION < 3 */
-
        Py_Finalize();
-       
 #endif /* PY_MAJOR_VERSION < 3 */
     }
     
@@ -388,14 +403,12 @@ int pcilib_py_load_script(pcilib_t *ctx, const char *script_name) {
     if (module) return 0;
 
     gstate = PyGILState_Ensure();
-
     pymodule = PyImport_ImportModule(module_name);
     if (!pymodule) {
-	pcilib_python_error("Error importing script (%s)", script_name);
 	PyGILState_Release(gstate);
+	pcilib_python_error("Error importing script (%s)", script_name);
 	return PCILIB_ERROR_FAILED;
     }
-    
     PyGILState_Release(gstate);
 
     module = (pcilib_script_t*)malloc(sizeof(pcilib_script_t));
@@ -433,8 +446,8 @@ int pcilib_py_get_transform_script_properties(pcilib_t *ctx, const char *script_
 	
     dict = PyModule_GetDict(module->module);
     if (!dict) {
-	pcilib_python_error("Error getting dictionary for script (%s)", script_name);
 	PyGILState_Release(gstate);
+	pcilib_python_error("Error getting dictionary for script (%s)", script_name);
 	return PCILIB_ERROR_FAILED;
     }