Add win32 support

Make adPython work on Windows. Change how adPython calls into the embedded Python to use PyGILState_Ensure() and PyGILState_Release() which is appropriate for calling from non-Python-created threads which is the case for adPython which is called from areaDetector-created threads. Without this change, adPython hangs on Windows. The Python calling idiom is documented in the following: * https://docs.python.org/2/c-api/init.html#non-python-created-threads * https://stackoverflow.com/a/4975906 * https://bugs.python.org/issue1720250#msg57619 Install the plugin scripts so that adPython can reference an installed pathname rather than a source pathname. Remove the automatic determination of the Python version, include pathnames, library pathnames, etc. by executing external commands from Make since it did not work on Windows. Replace that with explicit definitions in configure/CONFIG_SITE.
DiamondLightSource · Sep 12, 2018 · 9ed2568 · 9ed2568
1 parent a030175
commit 9ed2568
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 57 deletions.
diff --git a/adPythonApp/Makefile b/adPythonApp/Makefile
@@ -6,5 +6,6 @@ DIRS := $(DIRS) $(filter-out $(DIRS), $(wildcard *db*))
 DIRS := $(DIRS) $(filter-out $(DIRS), $(wildcard *Db*))
 DIRS := $(DIRS) $(filter-out $(DIRS), $(wildcard *opi*))
 DIRS := $(DIRS) $(filter-out $(DIRS), $(wildcard protocol))
+DIRS := $(DIRS) $(filter-out $(DIRS), $(wildcard scripts))
 include $(TOP)/configure/RULES_DIRS
 
diff --git a/adPythonApp/scripts/Makefile b/adPythonApp/scripts/Makefile
@@ -0,0 +1,26 @@
+TOP=../..
+include $(TOP)/configure/CONFIG
+#----------------------------------------
+#  ADD MACRO DEFINITIONS AFTER THIS LINE
+
+#----------------------------------------------------
+#  Install scripts
+PLUGINSCRIPTS += adPythonBarCode.py
+PLUGINSCRIPTS += adPythonCircle.py
+PLUGINSCRIPTS += adPythonCrystalDroplet.py
+PLUGINSCRIPTS += adPythonCrystalFeatureMatch.py
+PLUGINSCRIPTS += adPythonDataMatrix.py
+PLUGINSCRIPTS += adPythonFocus.py
+PLUGINSCRIPTS += adPythonGaussian2DFitter.py
+PLUGINSCRIPTS += adPythonMitegen.py
+PLUGINSCRIPTS += adPythonMorph.py
+PLUGINSCRIPTS += adPythonMxSampleDetect.py
+PLUGINSCRIPTS += adPythonPowerMean.py
+PLUGINSCRIPTS += adPythonRotate.py
+PLUGINSCRIPTS += adPythonTemplate.py
+PLUGINSCRIPTS += adPythonTransfer.py
+PLUGINSCRIPTS += transferclient.py
+
+include $(TOP)/configure/RULES
+#----------------------------------------
+#  ADD RULES AFTER THIS LINE
diff --git a/adPythonApp/src/Makefile b/adPythonApp/src/Makefile
@@ -8,11 +8,18 @@ include $(TOP)/configure/CONFIG
 
 # This tells the compiler to ignore errors generated by EPICS includes.  We need
 # this because the EPICS headers have non strict prototypes in places.
+ifdef OS_CLASS
+ifneq ($(OS_CLASS), WIN32)
 USR_CPPFLAGS += -isystem $(EPICS_BASE)/include
+endif
+endif
 
 # Nice and strict...
+USR_CXXFLAGS += -Wall
+ifdef OS_CLASS
+ifneq ($(OS_CLASS), WIN32)
 USR_CXXFLAGS += -Werror
-USR_CXXFLAGS += -Wall -Wextra
+USR_CXXFLAGS += -Wextra
 USR_CXXFLAGS += -Wno-unused-parameter
 USR_CXXFLAGS += -Wno-missing-field-initializers
 USR_CXXFLAGS += -Wundef
@@ -21,6 +28,8 @@ USR_CXXFLAGS += -Wcast-align
 USR_CXXFLAGS += -Wwrite-strings
 USR_CXXFLAGS += -Wredundant-decls
 USR_CXXFLAGS += -Wmissing-declarations
+endif
+endif
 
 LIBRARY_IOC += adPython
 
@@ -30,31 +39,27 @@ DBD += adPythonPlugin.dbd
 # The following are compiled and added to the support library
 adPython_SRCS += adPythonPlugin.cpp
 
-# Guess the python executable
-ifneq ($(PYTHON_PREFIX),)
-    PYTHON = $(PYTHON_PREFIX)/bin/python
-else
-    PYTHON = python
-endif
-
-# Get the version string of python
-PYTHON_VERSION = $(shell $(PYTHON) -c 'from distutils import sysconfig; \
-    print sysconfig.get_config_var("VERSION")')
-
 # link against python
 ifneq ($(PYTHON_PREFIX),)
     # user defined prefix, add it explicitly
-	USR_INCLUDES += -I$(PYTHON_PREFIX)/include/python$(PYTHON_VERSION)
-	python$(PYTHON_VERSION)_DIR = $(PYTHON_PREFIX)/lib
+	USR_INCLUDES += -I$(PYTHON_INCLUDE)
+	USR_INCLUDES += -I$(NUMPY_INCLUDE)
+	python$(PYTHON_VERSION)_DIR = $(PYTHON_LIB)
 	adPython_LIBS += python$(PYTHON_VERSION)
-	# add the numpy include path too, which is printed out when we run
-	# adPythonPlugin.py from the command line
-	USR_INCLUDES += -I$(shell $(PYTHON) ../adPythonPlugin.py)
 else
     # it's in a standard place
 	adPython_SYS_LIBS += python$(PYTHON_VERSION)
 endif
 
-USR_CXXFLAGS += -DDATADIRS=\"$(shell cd ..; pwd):$(shell cd ../../scripts; pwd)\"
+# add more libs
+adPython_LIBS += ADBase NDPlugin asyn
+adPython_LIBS += $(EPICS_BASE_IOC_LIBS)
+
+# location of plugin scripts
+USR_CXXFLAGS += -DDATADIRS=\"$(ADPYTHON_MODULE_PATH)\"
+
+# install plugin scripts
+PLUGINSCRIPTS += adPythonOffline.py
+PLUGINSCRIPTS += adPythonPlugin.py
 
 include $(TOP)/configure/RULES
diff --git a/adPythonApp/src/adPythonPlugin.cpp b/adPythonApp/src/adPythonPlugin.cpp
@@ -8,7 +8,6 @@
 #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
 #include "numpy/ndarrayobject.h"
 #include <stdio.h>
-#include <libgen.h>
 #include <epicsTime.h>
 #include "NDArray.h"
 #include "adPythonPlugin.h"
@@ -34,14 +33,16 @@
 #define Bad(errString) NoGood(errString, BAD)
 #define Ugly(errString) NoGood(errString, UGLY)
 
+// Used in setting the python module search path
+#ifdef _WIN32
+#define PATH_LIST_SEPARATOR ";"
+#else
+#define PATH_LIST_SEPARATOR ":"
+#endif
+
 // Used in error printing
 const char *driverName = "adPythonPlugin";
 
-// This holds the threadState of the thread that initialised python
-// We need it to get a handle on the interpreter so create a threadState
-// object for each port
-static PyThreadState *mainThreadState = NULL;
-
 adPythonPlugin::adPythonPlugin(const char *portNameArg, const char *filename,
                    const char *classname, int queueSize, int blockingCallbacks,
                    const char *NDArrayPort, int NDArrayAddr, int maxBuffers, 
@@ -81,24 +82,24 @@ adPythonPlugin::adPythonPlugin(const char *portNameArg, const char *filename,
  */
 void adPythonPlugin::initThreads()
 {
-    // First we tell python where to find adPythonPlugin.py and other scripts
-    char buffer[BIGBUFFER];
-    snprintf(buffer, sizeof(buffer), "PYTHONPATH=%s", DATADIRS);
-    putenv(buffer);
-
-    // Now we initialise python
+    PyGILState_STATE gstate;
+
+    // Initialise python
     if (!Py_IsInitialized()) {
         PyEval_InitThreads();
         Py_Initialize();
-
-        // Be sure to save thread state to release the GIL and give a handle
-        // on the interpreter to this and other ports
-        mainThreadState = PyEval_SaveThread();
+        // Release the GIL
+        PyEval_SaveThread();
     }
 
-    // Create a thread state just for us
-    this->threadState = PyThreadState_New(mainThreadState->interp);
-    PyEval_RestoreThread(this->threadState);
+    // Acquire the GIL and set up non-python-created thread access
+    gstate = PyGILState_Ensure();
+
+    // Tell python where to find adPythonPlugin.py and other scripts
+    char buffer[BIGBUFFER];
+    snprintf(buffer, sizeof(buffer), "%s%s%s", Py_GetPath(),
+        PATH_LIST_SEPARATOR, DATADIRS);
+    PySys_SetPath(buffer);
 
     // Import our supporting library
     this->importAdPythonModule();
@@ -117,8 +118,8 @@ void adPythonPlugin::initThreads()
         this->updateParamList(1);
     }
 
-    // Release the GIL and finish
-    this->threadState = PyEval_SaveThread();
+    // Release the GIL and tear down non-python-created thread access
+    PyGILState_Release(gstate);
 }
 
 /** Callback function that is called by the NDArray driver with new NDArray data
@@ -130,6 +131,8 @@ void adPythonPlugin::initThreads()
   * Called with this->lock taken
   */
 void adPythonPlugin::processCallbacks(NDArray *pArray) {
+    PyGILState_STATE gstate;
+
     // First call the base class method
     NDPluginDriver::processCallbacks(pArray);
 
@@ -143,8 +146,8 @@ void adPythonPlugin::processCallbacks(NDArray *pArray) {
     this->unlock();
     epicsMutexLock(this->dictMutex);
 
-    // Make sure we're allowed to use the python API
-    PyEval_RestoreThread(this->threadState);
+    // Acquire the GIL and set up non-python-created thread access
+    gstate = PyGILState_Ensure();
     this->lock(); 
 
     // Store the time at the beginning of processing for profiling 
@@ -181,9 +184,11 @@ void adPythonPlugin::processCallbacks(NDArray *pArray) {
     setDoubleParam(adPythonTime, epicsTimeDiffInSeconds(&end, &start)*1000);
     callParamCallbacks();
 
-    // release GIL and dict Mutex 
+    // Unlock
     this->unlock();   
-    this->threadState = PyEval_SaveThread();
+    // Release the GIL and tear down non-python-created thread access
+    PyGILState_Release(gstate);
+    // Release dict mutex
     epicsMutexUnlock(this->dictMutex);
 
     // Spit out the array
@@ -209,14 +214,15 @@ asynStatus adPythonPlugin::writeInt32(asynUser *pasynUser, epicsInt32 value) {
     int param = pasynUser->reason;
     if (param == adPythonLoad || 
             (this->nextParam && param >= adPythonUserParams[0])) {
+        PyGILState_STATE gstate;
         // We have to modify our python dict to match our param list
         // Note: to avoid deadlocks we should always take locks in order:
         //  dictMutex, then GIL, then this->lock
         // so unlock here to preserve this order
         this->unlock();
         epicsMutexLock(this->dictMutex);
-        // Make sure we're allowed to use the python API
-        PyEval_RestoreThread(this->threadState);
+        // Acquire the GIL and set up non-python-created thread access
+        gstate = PyGILState_Ensure();
         // Now call the bast class to write the value to the param list
         this->lock();        
         status |= NDPluginDriver::writeInt32(pasynUser, value);               
@@ -230,8 +236,9 @@ asynStatus adPythonPlugin::writeInt32(asynUser *pasynUser, epicsInt32 value) {
             // our param lib has changed, so update the dict and reprocess
             status |= this->updateParamDict();
         }
-        // release GIL and dict Mutex
-        this->threadState = PyEval_SaveThread();
+        // Release the GIL and tear down non-python-created thread access
+        PyGILState_Release(gstate);
+        // Release dict mutex
         epicsMutexUnlock(this->dictMutex);    
     } else {
         status |= NDPluginDriver::writeInt32(pasynUser, value);
@@ -252,21 +259,23 @@ asynStatus adPythonPlugin::writeFloat64(asynUser *pasynUser,
     int status = asynSuccess;
     int param = pasynUser->reason;
     if (this->nextParam && param >= adPythonUserParams[0]) {
+        PyGILState_STATE gstate;
         // We have to modify our python dict to match our param list
         // Note: to avoid deadlocks we should always take locks in order:
         //  dictMutex, then GIL, then this->lock
         // so unlock here to preserve this order
         this->unlock();
         epicsMutexLock(this->dictMutex);
-        // Make sure we're allowed to use the python API
-        PyEval_RestoreThread(this->threadState);
+        // Acquire the GIL and set up non-python-created thread access
+        gstate = PyGILState_Ensure();
         // Now call the bast class to write the value to the param list
         this->lock();        
         status |= NDPluginDriver::writeFloat64(pasynUser, value);                
         // our param lib has changed, so update the dict and reprocess
         status |= this->updateParamDict();
-        // release GIL and dict Mutex
-        this->threadState = PyEval_SaveThread();
+        // Release the GIL and tear down non-python-created thread access
+        PyGILState_Release(gstate);
+        // Release dict mutex
         epicsMutexUnlock(this->dictMutex);                        
     } else {
         status = NDPluginDriver::writeFloat64(pasynUser, value);
@@ -289,21 +298,23 @@ asynStatus adPythonPlugin::writeOctet(asynUser *pasynUser, const char *value,
     int status = asynSuccess;
     int param = pasynUser->reason;
     if (this->nextParam && param >= adPythonUserParams[0]) {
+        PyGILState_STATE gstate;
         // We have to modify our python dict to match our param list
         // Note: to avoid deadlocks we should always take locks in order:
         //  dictMutex, then GIL, then this->lock
         // so unlock here to preserve this order
         this->unlock();
         epicsMutexLock(this->dictMutex);
-        // Make sure we're allowed to use the python API
-        PyEval_RestoreThread(this->threadState);
+        // Acquire the GIL and set up non-python-created thread access
+        gstate = PyGILState_Ensure();
         // Now call the bast class to write the value to the param list
         this->lock();        
         status |= NDPluginDriver::writeOctet(pasynUser, value, maxChars, nActual);                
         // our param lib has changed, so update the dict and reprocess
         status |= this->updateParamDict();
-        // release GIL and dict Mutex
-        this->threadState = PyEval_SaveThread();
+        // Release the GIL and tear down non-python-created thread access
+        PyGILState_Release(gstate);
+        // Release dict mutex
         epicsMutexUnlock(this->dictMutex);       
     } else {
         status |= NDPluginDriver::writeOctet(pasynUser, value, maxChars, nActual);

diff --git a/adPythonApp/src/adPythonPlugin.h b/adPythonApp/src/adPythonPlugin.h
@@ -6,6 +6,10 @@
 #include <iocsh.h>
 #include <epicsExport.h>
 
+#if defined(_MSC_VER) && !defined(__func__)
+#define __func__ __FUNCTION__
+#endif
+
 // Max number of user parameters in a subclass
 #define NUSERPARAMS 100
 
@@ -58,7 +62,6 @@ class adPythonPlugin : public NDPluginDriver {
     NDAttributeList *pFileAttributes;
     int nextParam, pluginState;
     epicsMutexId dictMutex;
-    PyThreadState *threadState;
 };
 
 #endif
diff --git a/configure/CONFIG_PLUGINSCRIPTS b/configure/CONFIG_PLUGINSCRIPTS
@@ -0,0 +1,2 @@
+FILE_TYPE += PLUGINSCRIPTS
+INSTALL_PLUGINSCRIPTS = $(INSTALL_LOCATION)/plugin-scripts
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		FILE_TYPE += PLUGINSCRIPTS
		INSTALL_PLUGINSCRIPTS = $(INSTALL_LOCATION)/plugin-scripts