Skip to content

Commit

Permalink
Add win32 support
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jlmuir
jlmuir committed Sep 20, 2018
1 parent a030175 commit d0eb687
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 57 deletions.
1 change: 1 addition & 0 deletions adPythonApp/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

26 changes: 26 additions & 0 deletions adPythonApp/scripts/Makefile
View file
Original file line number Diff line number Diff line change
@@ -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
40 changes: 22 additions & 18 deletions adPythonApp/src/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -30,31 +39,26 @@ 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 += NDPlugin

# location of plugin scripts
USR_CXXFLAGS += -DDATADIRS=\"$(ADPYTHON_MODULE_PATH)\"

# install plugin scripts
PLUGINSCRIPTS += adPythonOffline.py
PLUGINSCRIPTS += adPythonPlugin.py

include $(TOP)/configure/RULES
85 changes: 48 additions & 37 deletions adPythonApp/src/adPythonPlugin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand All @@ -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);

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion adPythonApp/src/adPythonPlugin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -58,7 +62,6 @@ class adPythonPlugin : public NDPluginDriver {
NDAttributeList *pFileAttributes;
int nextParam, pluginState;
epicsMutexId dictMutex;
PyThreadState *threadState;
};

#endif
2 changes: 2 additions & 0 deletions configure/CONFIG_PLUGINSCRIPTS
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FILE_TYPE += PLUGINSCRIPTS
INSTALL_PLUGINSCRIPTS = $(INSTALL_LOCATION)/plugin-scripts
Loading

0 comments on commit d0eb687

Please sign in to comment.