# HG changeset patch
# User cannam
# Date 1205496135 0
# Node ID 134313c59d829fb1b823c9bde8149b9e310521e2
# Parent  211ebe55d5213b6d7ba1b5780f1c23c48d815ab4
* Add global mutex to PyPlugin -- all plugin method calls are strictly
  serialised in order to avoid problems with Python interpreter's lack of
  thread safety.

diff -r 211ebe55d521 -r 134313c59d82 Mutex.cpp
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/Mutex.cpp	Fri Mar 14 12:02:15 2008 +0000
@@ -0,0 +1,196 @@
+/* -*- c-basic-offset: 4 indent-tabs-mode: nil -*-  vi:set ts=8 sts=4 sw=4: */
+
+/*
+   Basic cross-platform mutex abstraction class.
+   This file copyright 2007 Chris Cannam.
+*/
+
+#include "Mutex.h"
+
+#include <iostream>
+
+#ifndef _WIN32
+#include <sys/time.h>
+#include <time.h>
+#endif
+
+using std::cerr;
+using std::endl;
+using std::string;
+
+#ifdef _WIN32
+
+Mutex::Mutex()
+#ifndef NO_THREAD_CHECKS
+    :
+    m_lockedBy(-1)
+#endif
+{
+    m_mutex = CreateMutex(NULL, FALSE, NULL);
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)GetCurrentThreadId() << ": Initialised mutex " << &m_mutex << endl;
+#endif
+}
+
+Mutex::~Mutex()
+{
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)GetCurrentThreadId() << ": Destroying mutex " << &m_mutex << endl;
+#endif
+    CloseHandle(m_mutex);
+}
+
+void
+Mutex::lock()
+{
+#ifndef NO_THREAD_CHECKS
+    DWORD tid = GetCurrentThreadId();
+    if (m_lockedBy == tid) {
+        cerr << "ERROR: Deadlock on mutex " << &m_mutex << endl;
+    }
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Want to lock mutex " << &m_mutex << endl;
+#endif
+    WaitForSingleObject(m_mutex, INFINITE);
+#ifndef NO_THREAD_CHECKS
+    m_lockedBy = tid;
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Locked mutex " << &m_mutex << endl;
+#endif
+}
+
+void
+Mutex::unlock()
+{
+#ifndef NO_THREAD_CHECKS
+    DWORD tid = GetCurrentThreadId();
+    if (m_lockedBy != tid) {
+        cerr << "ERROR: Mutex " << &m_mutex << " not owned by unlocking thread" << endl;
+        return;
+    }
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Unlocking mutex " << &m_mutex << endl;
+#endif
+#ifndef NO_THREAD_CHECKS
+    m_lockedBy = -1;
+#endif
+    ReleaseMutex(m_mutex);
+}
+
+bool
+Mutex::trylock()
+{
+#ifndef NO_THREAD_CHECKS
+    DWORD tid = GetCurrentThreadId();
+#endif
+    DWORD result = WaitForSingleObject(m_mutex, 0);
+    if (result == WAIT_TIMEOUT || result == WAIT_FAILED) {
+#ifdef DEBUG_MUTEX
+        cerr << "MUTEX DEBUG: " << (void *)tid << ": Mutex " << &m_mutex << " unavailable" << endl;
+#endif
+        return false;
+    } else {
+#ifndef NO_THREAD_CHECKS
+        m_lockedBy = tid;
+#endif
+#ifdef DEBUG_MUTEX
+        cerr << "MUTEX DEBUG: " << (void *)tid << ": Locked mutex " << &m_mutex << " (from trylock)" << endl;
+#endif
+        return true;
+    }
+}
+
+#else  /* !_WIN32 */
+
+Mutex::Mutex()
+#ifndef NO_THREAD_CHECKS
+    :
+    m_lockedBy(0),
+    m_locked(false)
+#endif
+{
+    pthread_mutex_init(&m_mutex, 0);
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)pthread_self() << ": Initialised mutex " << &m_mutex << endl;
+#endif
+}
+
+Mutex::~Mutex()
+{
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)pthread_self() << ": Destroying mutex " << &m_mutex << endl;
+#endif
+    pthread_mutex_destroy(&m_mutex);
+}
+
+void
+Mutex::lock()
+{
+#ifndef NO_THREAD_CHECKS
+    pthread_t tid = pthread_self();
+    if (m_locked && m_lockedBy == tid) {
+        cerr << "ERROR: Deadlock on mutex " << &m_mutex << endl;
+    }
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Want to lock mutex " << &m_mutex << endl;
+#endif
+    pthread_mutex_lock(&m_mutex);
+#ifndef NO_THREAD_CHECKS
+    m_lockedBy = tid;
+    m_locked = true;
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Locked mutex " << &m_mutex << endl;
+#endif
+}
+
+void
+Mutex::unlock()
+{
+#ifndef NO_THREAD_CHECKS
+    pthread_t tid = pthread_self();
+    if (!m_locked) {
+        cerr << "ERROR: Mutex " << &m_mutex << " not locked in unlock" << endl;
+        return;
+    } else if (m_lockedBy != tid) {
+        cerr << "ERROR: Mutex " << &m_mutex << " not owned by unlocking thread" << endl;
+        return;
+    }
+#endif
+#ifdef DEBUG_MUTEX
+    cerr << "MUTEX DEBUG: " << (void *)tid << ": Unlocking mutex " << &m_mutex << endl;
+#endif
+#ifndef NO_THREAD_CHECKS
+    m_locked = false;
+#endif
+    pthread_mutex_unlock(&m_mutex);
+}
+
+bool
+Mutex::trylock()
+{
+#ifndef NO_THREAD_CHECKS
+    pthread_t tid = pthread_self();
+#endif
+    if (pthread_mutex_trylock(&m_mutex)) {
+#ifdef DEBUG_MUTEX
+        cerr << "MUTEX DEBUG: " << (void *)tid << ": Mutex " << &m_mutex << " unavailable" << endl;
+#endif
+        return false;
+    } else {
+#ifndef NO_THREAD_CHECKS
+        m_lockedBy = tid;
+        m_locked = true;
+#endif
+#ifdef DEBUG_MUTEX
+        cerr << "MUTEX DEBUG: " << (void *)tid << ": Locked mutex " << &m_mutex << " (from trylock)" << endl;
+#endif
+        return true;
+    }
+}
+
+#endif
diff -r 211ebe55d521 -r 134313c59d82 Mutex.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/Mutex.h	Fri Mar 14 12:02:15 2008 +0000
@@ -0,0 +1,53 @@
+/* -*- c-basic-offset: 4 indent-tabs-mode: nil -*-  vi:set ts=8 sts=4 sw=4: */
+
+/*
+   Basic cross-platform mutex abstraction class.
+   This file copyright 2007 Chris Cannam.
+*/
+
+#ifndef _MUTEX_H_
+#define _MUTEX_H_
+
+#ifdef _WIN32
+#include <windows.h>
+#else
+#include <pthread.h>
+#endif
+
+class Mutex
+{
+public:
+    Mutex();
+    ~Mutex();
+
+    void lock();
+    void unlock();
+    bool trylock();
+
+private:
+#ifdef _WIN32
+    HANDLE m_mutex;
+#ifndef NO_THREAD_CHECKS
+    DWORD m_lockedBy;
+#endif
+#else
+    pthread_mutex_t m_mutex;
+#ifndef NO_THREAD_CHECKS
+    pthread_t m_lockedBy;
+    bool m_locked;
+#endif
+#endif
+};
+
+class MutexLocker
+{
+public:
+    MutexLocker(Mutex *);
+    ~MutexLocker();
+
+private:
+    Mutex *m_mutex;
+};
+
+#endif
+
diff -r 211ebe55d521 -r 134313c59d82 PyPlugScanner.cpp
--- a/PyPlugScanner.cpp	Wed Mar 12 12:42:19 2008 +0000
+++ b/PyPlugScanner.cpp	Fri Mar 14 12:02:15 2008 +0000
@@ -6,7 +6,6 @@
 */
 
 
-#include "/usr/include/python/Python.h"
 #include "PyPlugScanner.h"
 
 //#include <fstream>
@@ -198,6 +197,12 @@
 
 //Return correct plugin directories as per platform
 //Code taken from vamp-sdk/PluginHostAdapter.cpp
+
+//!!! It would probably be better to actually call
+// PluginHostAdapter::getPluginPath.  That would mean this "plugin"
+// needs to link against vamp-hostsdk, but that's probably acceptable
+// as it is sort of a host as well.
+
 std::vector<std::string>
 PyPlugScanner::getAllValidPath()
 {
diff -r 211ebe55d521 -r 134313c59d82 PyPlugScanner.h
--- a/PyPlugScanner.h	Wed Mar 12 12:42:19 2008 +0000
+++ b/PyPlugScanner.h	Fri Mar 14 12:02:15 2008 +0000
@@ -41,7 +41,7 @@
 #ifndef _VAMP_PYPLUG_SCANNER_H_
 #define _VAMP_PYPLUG_SCANNER_H_
 
-#include "/usr/include/python/Python.h"
+#include <Python.h>
 #include <iostream>
 #include <vector>
 #include <string>
@@ -70,4 +70,4 @@
 };
 
 #endif	
-	
\ No newline at end of file
+
diff -r 211ebe55d521 -r 134313c59d82 PyPlugin.cpp
--- a/PyPlugin.cpp	Wed Mar 12 12:42:19 2008 +0000
+++ b/PyPlugin.cpp	Fri Mar 14 12:02:15 2008 +0000
@@ -48,8 +48,7 @@
 
 */
 
-//#include "Python.h"
-#include "/usr/include/python/Python.h"
+#include <Python.h>
 #include "PyPlugin.h"
 
 #ifdef _WIN32
@@ -66,14 +65,14 @@
 using std::endl;
 using std::map;
 
-extern volatile bool mutex;
-
 // Maps to associate strings with enum values
 static std::map<std::string, eOutDescriptors> outKeys;
 static std::map<std::string, eSampleTypes> sampleKeys;
 static std::map<std::string, eFeatureFields> ffKeys;
 static std::map<std::string, p::eParmDescriptors> parmKeys;
 
+Mutex PyPlugin::m_pythonInterpreterMutex;
+
 void initMaps()
 {
 	outKeys["identifier"] = identifier;
@@ -170,33 +169,20 @@
 	m_class(pluginKey.substr(pluginKey.rfind(':')+1,pluginKey.size()-1)),
 	m_path((pluginKey.substr(0,pluginKey.rfind(pathsep))))
 {	
-
-/*TODO: this is a nasty way of ensuring the process is 
-finished before we create a new instance accessing the Python/C API.
-The Python/C API is not fully thread safe. 
-Calling into a python class while the process is doing heavy 
-computation may cause segmentation fault. 
-Manipulating the GIL and thread states only gave me a grief so far.*/
-	
-	if (mutex) {
-		cerr << "PyPlugin::PyPlugin:" << m_class 
-		<< " Warning: Waiting for clear signal from parallel process." << endl;
-		while (mutex) { }
-	}	
 }
 
 PyPlugin::~PyPlugin()
 {
 	cerr << "PyPlugin::PyPlugin:" << m_class 
 	<< " Instance deleted." << endl;
-	//for safety only. has been cleared after process.
-	mutex = false;	
 }
 
 
 string
 PyPlugin::getIdentifier() const
 {	
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getIdentifier"; 
 	cerr << "[call] " << method << endl;
 	string rString="VampPy-x";
@@ -226,6 +212,7 @@
 string
 PyPlugin::getName() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
 
 	char method[]="getName";
 	cerr << "[call] " << method << endl;
@@ -254,6 +241,8 @@
 string
 PyPlugin::getDescription() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getDescription";
 	cerr << "[call] " << method << endl;
 	string rString="Not given. (Hint: Implement getDescription method.)";
@@ -281,6 +270,8 @@
 string
 PyPlugin::getMaker() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getMaker";
 	cerr << "[call] " << method << endl;
 	string rString="Generic VamPy Plugin.";
@@ -314,6 +305,8 @@
 string
 PyPlugin::getCopyright() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getCopyright";
 	cerr << "[call] " << method << endl;
 	string rString="BSD License";
@@ -343,6 +336,8 @@
 bool
 PyPlugin::initialise(size_t channels, size_t stepSize, size_t blockSize)
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
     if (channels < getMinChannelCount() ||
 	channels > getMaxChannelCount()) return false;
 
@@ -390,11 +385,14 @@
 void
 PyPlugin::reset()
 {
+	//!!! implement this!
     m_previousSample = 0.0f;
 }
 
 PyPlugin::InputDomain PyPlugin::getInputDomain() const 
 { 
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getInputDomain";
 	cerr << "[call] " << method << endl;
 	PyPlugin::InputDomain rValue = TimeDomain; // TimeDomain
@@ -420,6 +418,8 @@
 
 size_t PyPlugin::getPreferredBlockSize() const 
 { 
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getPreferredBlockSize";
 	cerr << "[call] " << method << endl;
 	size_t rValue=0; //not set by default
@@ -443,6 +443,8 @@
 //size_t PyPlugin::getPreferredStepSize() const { return 0; }
 size_t PyPlugin::getPreferredStepSize() const 
 { 
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getPreferredStepSize";
 	cerr << "[call] " << method << endl;
 	size_t rValue=0; //not set by default
@@ -465,6 +467,8 @@
 
 size_t PyPlugin::getMinChannelCount() const 
 { 
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getMinChannelCount";
 	cerr << "[call] " << method << endl;
 	size_t rValue=1; //default value
@@ -487,6 +491,8 @@
 
 size_t PyPlugin::getMaxChannelCount() const 
 { 
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getMaxChannelCount";	
 	cerr << "[call] " << method << endl;
 	size_t rValue=1; //default value
@@ -511,6 +517,8 @@
 PyPlugin::OutputList
 PyPlugin::getOutputDescriptors() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	//PyEval_AcquireThread(newThreadState);
 	OutputList list;
 	OutputDescriptor od;
@@ -612,6 +620,8 @@
 PyPlugin::ParameterList
 PyPlugin::getParameterDescriptors() const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	ParameterList list;
 	ParameterDescriptor pd;
 	char method[]="getParameterDescriptors";
@@ -693,6 +703,8 @@
 
 void PyPlugin::setParameter(std::string paramid, float newval)
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="setParameter";
 	cerr << "[call] " << method << endl;
 
@@ -722,6 +734,8 @@
 
 float PyPlugin::getParameter(std::string paramid) const
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	char method[]="getParameter";
 	cerr << "[call] " << method << endl;
 	float rValue = 0.0f;
@@ -763,12 +777,13 @@
 PyPlugin::process(const float *const *inputBuffers,
                       Vamp::RealTime timestamp)
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
     if (m_stepSize == 0) {
 	cerr << "ERROR: PyPlugin::process: "
 	     << "Plugin has not been initialised" << endl;
 	return FeatureSet();
     }
-	mutex = true;
 	static char method[]="process";
 
 #ifdef _DEBUG	
@@ -812,7 +827,6 @@
 			}
 			Py_CLEAR(pyMethod);
 			Py_CLEAR(pyOutputList);
-			mutex = false;
 			return FeatureSet();
 		}
 			
@@ -887,11 +901,9 @@
 
 		}//for i = FeatureSet
 		Py_CLEAR(pyOutputList);
-		mutex = false;
 		return returnFeatures;
 
 	}//if (has_attribute)
-	mutex = false;
 	return FeatureSet(); 
 }
 
@@ -900,11 +912,12 @@
 PyPlugin::FeatureSet
 PyPlugin::getRemainingFeatures()
 {
+	MutexLocker locker(&m_pythonInterpreterMutex);
+
 	static char method[]="getRemainingFeatures";
 	cerr << "[call] " << method << endl;
 
 	//check if the method is implemented
-	mutex = true;
 	if ( ! PyObject_HasAttrString(m_pyInstance,method) ) {
 		return FeatureSet(); 
 		}
@@ -926,7 +939,6 @@
 			}
 			Py_CLEAR(pyMethod);
 			Py_CLEAR(pyOutputList);
-			mutex = false;
 			return FeatureSet();
 		}
 		Py_DECREF(pyMethod);
@@ -979,7 +991,6 @@
 			}// for j 			
 		}//for i 
 		Py_CLEAR(pyOutputList);
-		mutex = false;
 		return returnFeatures;
 }
 
diff -r 211ebe55d521 -r 134313c59d82 PyPlugin.h
--- a/PyPlugin.h	Wed Mar 12 12:42:19 2008 +0000
+++ b/PyPlugin.h	Fri Mar 14 12:02:15 2008 +0000
@@ -40,7 +40,9 @@
 #define _PYTHON_WRAPPER_PLUGIN_H_
 
 #include "vamp-sdk/Plugin.h"
-#include "/usr/include/python/Python.h"
+#include <Python.h>
+
+#include "Mutex.h"
 
 //fields in OutputDescriptor
 enum eOutDescriptors {
@@ -130,6 +132,8 @@
 	std::string m_plugin;
 	std::string m_class;
 	std::string m_path;
+
+	static Mutex m_pythonInterpreterMutex;
 };
 
 
diff -r 211ebe55d521 -r 134313c59d82 pyvamp-main.cpp
--- a/pyvamp-main.cpp	Wed Mar 12 12:42:19 2008 +0000
+++ b/pyvamp-main.cpp	Fri Mar 14 12:02:15 2008 +0000
@@ -40,8 +40,7 @@
  * Copyright 2008, George Fazekas.
 */
 
-//#include "Python.h"
-#include "/usr/include/python/Python.h"
+#include <Python.h>
 #include "vamp/vamp.h"
 #include "vamp-sdk/PluginAdapter.h"
 #include "PyPlugScanner.h"