From 2a2a530c627f12c3edaa82285c9cd3586a9a5d95 Mon Sep 17 00:00:00 2001 From: Dick Hollenbeck <dick@softplc.com> Date: Mon, 11 Mar 2013 03:09:48 -0500 Subject: [PATCH] improvements to python's GIL acquisition and release, but not done yet, since I think the GIL needs to be acquired even when not involving wxPython. --- pcbnew/pcbframe.cpp | 7 +- pcbnew/scripting/pcbnew_footprint_wizards.cpp | 247 +++++++++--------- pcbnew/scripting/pcbnew_footprint_wizards.h | 8 +- scripting/python_scripting.cpp | 35 ++- scripting/python_scripting.h | 58 ++-- 5 files changed, 179 insertions(+), 176 deletions(-) diff --git a/pcbnew/pcbframe.cpp b/pcbnew/pcbframe.cpp index 3adb6e351e..3378c576e3 100644 --- a/pcbnew/pcbframe.cpp +++ b/pcbnew/pcbframe.cpp @@ -55,7 +55,7 @@ #include <dialog_helpers.h> #include <convert_from_iu.h> -#ifdef KICAD_SCRIPTING +#if defined(KICAD_SCRIPTING) || defined(KICAD_SCRIPTING_WXPYTHON) #include <python_scripting.h> #endif @@ -410,7 +410,7 @@ PCB_EDIT_FRAME::PCB_EDIT_FRAME( wxWindow* parent, const wxString& title, wxAuiPaneInfo( mesg ).Name( wxT( "MsgPanel" ) ).Bottom().Layer(10) ); - #ifdef KICAD_SCRIPTING_WXPYTHON +#ifdef KICAD_SCRIPTING_WXPYTHON // Add the scripting panel EDA_PANEINFO pythonAuiInfo; pythonAuiInfo.ScriptingToolbarPane(); @@ -424,8 +424,7 @@ PCB_EDIT_FRAME::PCB_EDIT_FRAME( wxWindow* parent, const wxString& title, pythonAuiInfo.Name( wxT( "PythonPanel" ) ).Bottom().Layer(9) ); m_pythonPanelHidden = true; - #endif - +#endif ReFillLayerWidget(); // this is near end because contents establish size diff --git a/pcbnew/scripting/pcbnew_footprint_wizards.cpp b/pcbnew/scripting/pcbnew_footprint_wizards.cpp index 8317304a8e..5f3c4345dc 100644 --- a/pcbnew/scripting/pcbnew_footprint_wizards.cpp +++ b/pcbnew/scripting/pcbnew_footprint_wizards.cpp @@ -8,190 +8,190 @@ #include <stdio.h> -PYTHON_FOOTPRINT_WIZARD::PYTHON_FOOTPRINT_WIZARD(PyObject *aWizard) + +PYTHON_FOOTPRINT_WIZARD::PYTHON_FOOTPRINT_WIZARD( PyObject* aWizard ) { + PyLOCK lock; + this->m_PyWizard= aWizard; Py_XINCREF( aWizard ); } + PYTHON_FOOTPRINT_WIZARD::~PYTHON_FOOTPRINT_WIZARD() { + PyLOCK lock; + Py_XDECREF( this->m_PyWizard ); } -PyObject* PYTHON_FOOTPRINT_WIZARD::CallMethod(const char* aMethod, PyObject *aArglist) + +PyObject* PYTHON_FOOTPRINT_WIZARD::CallMethod( const char* aMethod, PyObject* aArglist ) { - PyObject *pFunc; + PyLOCK lock; - /* pFunc is a new reference to the desired method */ - pFunc = PyObject_GetAttrString( this->m_PyWizard, aMethod ); + // pFunc is a new reference to the desired method + PyObject* pFunc = PyObject_GetAttrString( this->m_PyWizard, aMethod ); - if ( pFunc && PyCallable_Check( pFunc ) ) + if( pFunc && PyCallable_Check( pFunc ) ) { - PyObject *result; + PyObject* result = PyObject_CallObject( pFunc, aArglist ); - PY_BLOCK_THREADS( blocked ); - - result = PyObject_CallObject( pFunc, aArglist ); - - if ( PyErr_Occurred() ) + if( PyErr_Occurred() ) { - PyObject *t, *v, *b; + PyObject* t; + PyObject* v; + PyObject* b; + PyErr_Fetch( &t, &v, &b ); printf ( "calling %s()\n", aMethod ); - printf ( "Exception: %s\n", PyString_AsString( PyObject_Str(v) ) ); - printf ( " : %s\n", PyString_AsString( PyObject_Str(b) ) ); + printf ( "Exception: %s\n", PyString_AsString( PyObject_Str( v ) ) ); + printf ( " : %s\n", PyString_AsString( PyObject_Str( b ) ) ); } - PY_UNBLOCK_THREADS( blocked ); - - if ( result ) + if( result ) { Py_XDECREF( pFunc ); return result; } - } else { printf( "method not found, or not callable: %s\n", aMethod ); } - if ( pFunc ) + if( pFunc ) Py_XDECREF( pFunc ); return NULL; } -wxString PYTHON_FOOTPRINT_WIZARD::CallRetStrMethod( const char* aMethod, PyObject *aArglist ) -{ - wxString ret; - ret.Clear(); - PyObject *result = CallMethod( aMethod, aArglist ); - if ( result ) +wxString PYTHON_FOOTPRINT_WIZARD::CallRetStrMethod( const char* aMethod, PyObject* aArglist ) +{ + wxString ret; + PyLOCK lock; + + PyObject* result = CallMethod( aMethod, aArglist ); + if( result ) { - PY_BLOCK_THREADS( blocked ); - const char *str_res = PyString_AsString( result ); + const char* str_res = PyString_AsString( result ); ret = wxString::FromUTF8( str_res ); Py_DECREF( result ); - - PY_UNBLOCK_THREADS( blocked ); } return ret; } + wxArrayString PYTHON_FOOTPRINT_WIZARD::CallRetArrayStrMethod - ( const char *aMethod, PyObject *aArglist ) + ( const char* aMethod, PyObject* aArglist ) { - PyObject *result, *element; wxArrayString ret; - wxString str_item; + wxString str_item; + PyLOCK lock; - result = CallMethod( aMethod, aArglist ); + PyObject* result = CallMethod( aMethod, aArglist ); - if ( result ) + if( result ) { - if ( !PyList_Check(result) ) + if( !PyList_Check( result ) ) { Py_DECREF( result ); - ret.Add( wxT("PYTHON_FOOTPRINT_WIZARD::CallRetArrayStrMethod, result is not a list"), 1 ); + ret.Add( wxT( "PYTHON_FOOTPRINT_WIZARD::CallRetArrayStrMethod, result is not a list" ), 1 ); return ret; } - PY_BLOCK_THREADS( blocked ); - int list_size = PyList_Size( result ); for ( int n=0; n<list_size; n++ ) { - element = PyList_GetItem( result, n ); + PyObject* element = PyList_GetItem( result, n ); + + const char* str_res = PyString_AsString( element ); - const char *str_res = PyString_AsString( element ); str_item = wxString::FromUTF8( str_res ); ret.Add( str_item, 1 ); } Py_DECREF( result ); - - PY_UNBLOCK_THREADS( blocked ); } - - return ret; - } + wxString PYTHON_FOOTPRINT_WIZARD::GetName() { + PyLOCK lock; + return CallRetStrMethod( "GetName" ); } + wxString PYTHON_FOOTPRINT_WIZARD::GetImage() { + PyLOCK lock; + return CallRetStrMethod( "GetImage" ); } + wxString PYTHON_FOOTPRINT_WIZARD::GetDescription() { + PyLOCK lock; + return CallRetStrMethod( "GetDescription" ); } + int PYTHON_FOOTPRINT_WIZARD::GetNumParameterPages() { - int ret = 0; - PyObject *result; + int ret = 0; + PyLOCK lock; - /* Time to call the callback */ - result = CallMethod( "GetNumParameterPages" , NULL ); + // Time to call the callback + PyObject* result = CallMethod( "GetNumParameterPages" , NULL ); - if (result) + if( result ) { - if ( !PyInt_Check( result ) ) + if( !PyInt_Check( result ) ) return -1; - PY_BLOCK_THREADS( blocked ); - ret = PyInt_AsLong( result ); Py_DECREF( result ); - - PY_UNBLOCK_THREADS( blocked ); } + return ret; } + wxString PYTHON_FOOTPRINT_WIZARD::GetParameterPageName( int aPage ) { - wxString ret; - PyObject *arglist; - PyObject *result; + wxString ret; + PyLOCK lock; + + // Time to call the callback + PyObject* arglist = Py_BuildValue( "( i )", aPage ); + PyObject* result = CallMethod( "GetParameterPageName", arglist ); - /* Time to call the callback */ - arglist = Py_BuildValue( "(i)", aPage ); - result = CallMethod( "GetParameterPageName", arglist ); Py_DECREF( arglist ); - if ( result ) + if( result ) { - PY_BLOCK_THREADS( blocked ); - - const char *str_res = PyString_AsString( result ); + const char* str_res = PyString_AsString( result ); ret = wxString::FromUTF8( str_res ); Py_DECREF( result ); - - PY_UNBLOCK_THREADS( blocked ); } return ret; } + wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterNames( int aPage ) { - - PyObject *arglist; wxArrayString ret; + PyLOCK lock; - arglist = Py_BuildValue( "(i)", aPage ); + PyObject* arglist = Py_BuildValue( "( i )", aPage ); ret = CallRetArrayStrMethod( "GetParameterNames", arglist ); Py_DECREF( arglist ); @@ -199,53 +199,51 @@ wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterNames( int aPage ) { wxString rest; wxString item = ret[i]; - if ( item.StartsWith( wxT("*"), &rest ) ) + if( item.StartsWith( wxT( "*" ), &rest ) ) { ret[i]=rest; } } - return ret; } + wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterTypes( int aPage ) { + wxArrayString ret; + PyLOCK lock; - PyObject *arglist; - wxArrayString ret; - - arglist = Py_BuildValue( "(i)", aPage ); + PyObject* arglist = Py_BuildValue( "( i )", aPage ); ret = CallRetArrayStrMethod( "GetParameterNames", arglist ); - Py_DECREF(arglist); + Py_DECREF( arglist ); for ( unsigned i=0; i<ret.GetCount(); i++ ) { wxString rest; wxString item = ret[i]; - if ( item.StartsWith( wxT("*"), &rest ) ) + if( item.StartsWith( wxT( "*" ), &rest ) ) { - ret[i]=wxT( "UNITS" ); /* units */ + ret[i] = wxT( "UNITS" ); // units } else { - ret[i]=wxT( "IU" ); /* internal units */ + ret[i] = wxT( "IU" ); // internal units } } - return ret; } - wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterValues( int aPage ) { - PyObject *arglist; - wxArrayString ret; + PyLOCK lock; + + PyObject* arglist = Py_BuildValue( "( i )", aPage ); + + wxArrayString ret = CallRetArrayStrMethod( "GetParameterValues", arglist ); - arglist = Py_BuildValue( "(i)", aPage ); - ret = CallRetArrayStrMethod( "GetParameterValues", arglist ); Py_DECREF( arglist ); return ret; @@ -253,84 +251,83 @@ wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterValues( int aPage ) wxArrayString PYTHON_FOOTPRINT_WIZARD::GetParameterErrors( int aPage ) { - PyObject *arglist; - wxArrayString ret; + PyLOCK lock; - arglist = Py_BuildValue( "(i)", aPage ); - ret = CallRetArrayStrMethod( "GetParameterErrors", arglist ); + PyObject* arglist = Py_BuildValue( "( i )", aPage ); + + wxArrayString ret = CallRetArrayStrMethod( "GetParameterErrors", arglist ); Py_DECREF( arglist ); return ret; } + wxString PYTHON_FOOTPRINT_WIZARD::SetParameterValues( int aPage, wxArrayString& aValues ) { - int len = aValues.size(); - PyObject *py_list; - py_list = PyList_New( len ); + PyLOCK lock; + + PyObject* py_list = PyList_New( len ); for ( int i=0; i<len ; i++ ) { wxString str = aValues[i]; - PyObject *py_str = PyString_FromString( (const char*)str.mb_str() ); + PyObject* py_str = PyString_FromString( ( const char* )str.mb_str() ); PyList_SetItem( py_list, i, py_str ); } - PyObject *arglist; + PyObject* arglist; - arglist = Py_BuildValue( "(i,O)", aPage, py_list ); + arglist = Py_BuildValue( "( i,O )", aPage, py_list ); wxString res = CallRetStrMethod( "SetParameterValues", arglist ); Py_DECREF( arglist ); return res; } -/* this is a SWIG function declaration -from module.i*/ -MODULE *PyModule_to_MODULE(PyObject *obj0); -MODULE *PYTHON_FOOTPRINT_WIZARD::GetModule() +// this is a SWIG function declaration -from module.i +MODULE* PyModule_to_MODULE( PyObject* obj0 ); + + +MODULE* PYTHON_FOOTPRINT_WIZARD::GetModule() { - PyObject *result, *obj; - result = CallMethod( "GetModule", NULL ); + PyLOCK lock; - if (!result) + PyObject* result = CallMethod( "GetModule", NULL ); + + if( !result ) return NULL; - PY_BLOCK_THREADS( blocked ); + PyObject* obj = PyObject_GetAttrString( result, "this" ); - obj = PyObject_GetAttrString( result, "this" ); - - if ( PyErr_Occurred() ) + if( PyErr_Occurred() ) { /* PyObject *t, *v, *b; - PyErr_Fetch(&t, &v, &b); - printf ("calling GetModule()\n"); - printf ("Exception: %s\n",PyString_AsString(PyObject_Str(v))); - printf (" : %s\n",PyString_AsString(PyObject_Str(b))); + PyErr_Fetch( &t, &v, &b ); + printf ( "calling GetModule()\n" ); + printf ( "Exception: %s\n",PyString_AsString( PyObject_Str( v ) ) ); + printf ( " : %s\n",PyString_AsString( PyObject_Str( b ) ) ); */ PyErr_Print(); } - PY_UNBLOCK_THREADS( blocked ); - MODULE *mod = PyModule_to_MODULE( obj ); + MODULE* mod = PyModule_to_MODULE( obj ); return mod; } -void PYTHON_FOOTPRINT_WIZARDS::register_wizard(PyObject* aPyWizard) +void PYTHON_FOOTPRINT_WIZARDS::register_wizard( PyObject* aPyWizard ) { - PYTHON_FOOTPRINT_WIZARD *fw; - - fw = new PYTHON_FOOTPRINT_WIZARD( aPyWizard ); + PYTHON_FOOTPRINT_WIZARD* fw = new PYTHON_FOOTPRINT_WIZARD( aPyWizard ); //printf( "Registered python footprint wizard '%s'\n", - // (const char*)fw->GetName().mb_str() - // ); + // ( const char* )fw->GetName().mb_str() + // ); // this get the wizard registered in the common // FOOTPRINT_WIZARDS class @@ -338,17 +335,15 @@ void PYTHON_FOOTPRINT_WIZARDS::register_wizard(PyObject* aPyWizard) fw->register_wizard(); #if 0 - /* just to test if it works correctly */ + // just to test if it works correctly int pages = fw->GetNumParameterPages(); - printf(" %d pages\n",pages); + printf( " %d pages\n",pages ); - for (int n=0; n<pages; n++) + for ( int n=0; n<pages; n++ ) { - printf(" page %d->'%s'\n",n, - (const char*)fw->GetParameterPageName(n).mb_str()); + printf( " page %d->'%s'\n",n, + ( const char* )fw->GetParameterPageName( n ).mb_str() ); } #endif } - - diff --git a/pcbnew/scripting/pcbnew_footprint_wizards.h b/pcbnew/scripting/pcbnew_footprint_wizards.h index 0799e5feb0..af9af429e9 100644 --- a/pcbnew/scripting/pcbnew_footprint_wizards.h +++ b/pcbnew/scripting/pcbnew_footprint_wizards.h @@ -17,14 +17,14 @@ class PYTHON_FOOTPRINT_WIZARD: public FOOTPRINT_WIZARD PyObject *m_PyWizard; PyObject *CallMethod( const char *aMethod, PyObject *aArglist=NULL ); wxString CallRetStrMethod( const char *aMethod, PyObject *aArglist=NULL ); - wxArrayString CallRetArrayStrMethod( const char *aMethod, + wxArrayString CallRetArrayStrMethod( const char *aMethod, PyObject *aArglist=NULL ); public: PYTHON_FOOTPRINT_WIZARD( PyObject *wizard ); ~PYTHON_FOOTPRINT_WIZARD(); wxString GetName(); - wxString GetImage(); + wxString GetImage(); wxString GetDescription(); int GetNumParameterPages(); wxString GetParameterPageName( int aPage ); @@ -33,11 +33,11 @@ public: wxArrayString GetParameterValues( int aPage ); wxArrayString GetParameterErrors( int aPage ); wxString SetParameterValues( int aPage, wxArrayString& aValues ); //< must return "OK" or error description - MODULE *GetModule(); + MODULE* GetModule(); }; -class PYTHON_FOOTPRINT_WIZARDS +class PYTHON_FOOTPRINT_WIZARDS { public: static void register_wizard( PyObject *aPyWizard ); diff --git a/scripting/python_scripting.cpp b/scripting/python_scripting.cpp index dcee9229aa..b929b7b308 100644 --- a/scripting/python_scripting.cpp +++ b/scripting/python_scripting.cpp @@ -45,8 +45,8 @@ extern "C" void init_kicad( void ); extern "C" void init_pcbnew( void ); -#define EXTRA_PYTHON_MODULES 10 // this is the number of python - // modules that we want to add into the list +#define EXTRA_PYTHON_MODULES 10 // this is the number of python + // modules that we want to add into the list /* python inittab that links module names to module init functions @@ -126,11 +126,10 @@ static void swigSwitchPythonBuiltin() * */ -PyThreadState *g_PythonMainTState; +PyThreadState* g_PythonMainTState; bool pcbnewInitPythonScripting() { - swigAddBuiltin(); // add builtin functions swigAddModules(); // add our own modules swigSwitchPythonBuiltin(); // switch the python builtin modules to our new list @@ -158,16 +157,18 @@ bool pcbnewInitPythonScripting() // load pcbnew inside python, and load all the user plugins, TODO: add system wide plugins - PY_BLOCK_THREADS( blocked ); #endif - PyRun_SimpleString( "import sys\n" - "sys.path.append(\".\")\n" - "import pcbnew\n" - "pcbnew.LoadPlugins()" - ); + { + PyLOCK lock; + + PyRun_SimpleString( "import sys\n" + "sys.path.append(\".\")\n" + "import pcbnew\n" + "pcbnew.LoadPlugins()" + ); + } - PY_UNBLOCK_THREADS( blocked ); return true; } @@ -180,7 +181,7 @@ void pcbnewFinishPythonScripting() } -#ifdef KICAD_SCRIPTING_WXPYTHON +#if defined(KICAD_SCRIPTING_WXPYTHON) void RedirectStdio() { @@ -193,9 +194,8 @@ import wx\n\ output = wx.PyOnDemandOutputWindow()\n\ c sys.stderr = output\n"; - PY_BLOCK_THREADS( blocked ); + PyLOCK lock; PyRun_SimpleString( python_redirect ); - PY_UNBLOCK_THREADS( blocked ); } @@ -228,7 +228,7 @@ def makeWindow(parent):\n\ PyObject* result; // As always, first grab the GIL - PY_BLOCK_THREADS( blocked ); + PyLOCK lock; // Now make a dictionary to serve as the global namespace when the code is // executed. Put a reference to the builtins module in it. @@ -245,7 +245,6 @@ def makeWindow(parent):\n\ if( !result ) { PyErr_Print(); - PY_UNBLOCK_THREADS( blocked ); return NULL; } Py_DECREF(result); @@ -285,10 +284,6 @@ def makeWindow(parent):\n\ Py_DECREF( globals ); Py_DECREF( tuple ); - // Finally, after all Python stuff is done, release the GIL - PY_UNBLOCK_THREADS( blocked ); - return window; } #endif - diff --git a/scripting/python_scripting.h b/scripting/python_scripting.h index 2964157668..7307965159 100644 --- a/scripting/python_scripting.h +++ b/scripting/python_scripting.h @@ -3,43 +3,57 @@ // undefs explained here: https://bugzilla.redhat.com/show_bug.cgi?id=427617 - #ifdef _POSIX_C_SOURCE - #undef _POSIX_C_SOURCE - #endif - #ifdef _XOPEN_SOURCE - #undef _XOPEN_SOURCE - #endif - - #include <Python.h> - #ifndef NO_WXPYTHON_EXTENSION_HEADERS - #ifdef KICAD_SCRIPTING_WXPYTHON - #include <wx/wxPython/wxPython.h> - #endif - #endif +#ifdef _POSIX_C_SOURCE + #undef _POSIX_C_SOURCE +#endif +#ifdef _XOPEN_SOURCE + #undef _XOPEN_SOURCE +#endif +#include <Python.h> +#ifndef NO_WXPYTHON_EXTENSION_HEADERS +#ifdef KICAD_SCRIPTING_WXPYTHON + #include <wx/wxPython/wxPython.h> +#endif +#endif /* Function pcbnewInitPythonScripting - * Initializes the Python engine inside pcbnew + * Initializes the Python engine inside pcbnew */ bool pcbnewInitPythonScripting(); void pcbnewFinishPythonScripting(); - #ifdef KICAD_SCRIPTING_WXPYTHON -void RedirectStdio(); -wxWindow* CreatePythonShellWindow(wxWindow* parent); +void RedirectStdio(); +wxWindow* CreatePythonShellWindow( wxWindow* parent ); + +class PyLOCK +{ + wxPyBlock_t b; + +public: + + // @todo, find out why these are wxPython specific. We need the GIL regardless. + // Should never assume python will only have one thread calling it. + PyLOCK() { b = wxPyBeginBlockThreads(); } + ~PyLOCK() { wxPyEndBlockThreads( b ); } +}; -#define PY_BLOCK_THREADS(name) wxPyBlock_t name = wxPyBeginBlockThreads() -#define PY_UNBLOCK_THREADS(name) wxPyEndBlockThreads(name) #else -#define PY_BLOCK_THREADS(name) -#define PY_UNBLOCK_THREADS(name) +class PyLOCK +{ +public: + // @todo: this is wrong, python docs clearly say we need the GIL, + // irrespective of wxPython. + PyLOCK() {} + ~PyLOCK() {} +}; #endif -#endif +#endif // __PYTHON_SCRIPTING_H