This fixes the fact that python leaves objects hanging around. Also, …

…it prevents the accidental callback from arriving on python objects where the interpreter has been long closed. closes #13624

xbmc/interfaces/legacy/CallbackHandler.cpp

@@ -84,7 +84,7 @@ namespace XBMCAddon
       AddonClass::Ref<AsynchCallbackMessage> p(*iter);
 
       // only call when we are in the right thread state
-      if(p->handler->isThreadStateOk())
+      if(p->handler->isStateOk(p->cb->getObject()))
       {
         // remove it from the queue. No matter what we're done with
         //  this. Even if it doesn't execute for some reason.
@@ -140,7 +140,7 @@ namespace XBMCAddon
     {
       AddonClass::Ref<AsynchCallbackMessage> p(*iter);
 
-      if(p->handler->shouldRemoveCallback(userData))
+      if(p->handler->shouldRemoveCallback(p->cb->getObject(),userData))
       {
 #ifdef ENABLE_TRACE_API
         CLog::Log(LOGDEBUG,"%sNEWADDON removing callback 0x%lx for PyThreadState 0x%lx from queue", _tg.getSpaces(),(long)(p->cb.get()) ,(long)userData);

xbmc/interfaces/legacy/CallbackHandler.h

@@ -64,8 +64,8 @@ namespace XBMCAddon
     static void makePendingCalls();
     static void clearPendingCalls(void* userData);
 
-    virtual bool isThreadStateOk() =  0;
-    virtual bool shouldRemoveCallback(void* userData) = 0;
+    virtual bool isStateOk(AddonClass* obj) =  0;
+    virtual bool shouldRemoveCallback(AddonClass* obj, void* userData) = 0;
   };
 
 }

xbmc/interfaces/python/CallbackHandler.cpp

@@ -20,6 +20,7 @@
  */
 
 #include "CallbackHandler.h"
+#include "LanguageHook.h"
 
 namespace XBMCAddon
 {
@@ -42,10 +43,18 @@ namespace XBMCAddon
      * Now we are answering the question as to whether or not we are in the
      *  PyThreadState that we were in when we started.
      */
-    bool PythonCallbackHandler::isThreadStateOk()
+    bool PythonCallbackHandler::isStateOk(AddonClass* obj)
     {
       TRACE;
-      return objectThreadState == PyThreadState_Get();
+      PyThreadState* state = PyThreadState_Get();
+      if (objectThreadState == state)
+      {
+        // make sure the interpreter is still active.
+        AddonClass::Ref<XBMCAddon::Python::LanguageHook> lh(XBMCAddon::Python::LanguageHook::getIfExists(state->interp));
+        if (lh.isNotNull() && lh->hasRegisteredClass(obj))
+          return true;
+      }
+      return false;
     }
 
     /**
@@ -55,10 +64,15 @@ namespace XBMCAddon
      * TODO: This is a stupid way to get this information back to the handler.
      *  there should be a more language neutral means.
      */
-    bool PythonCallbackHandler::shouldRemoveCallback(void* threadState)
+    bool PythonCallbackHandler::shouldRemoveCallback(AddonClass* obj, void* threadState)
     {
       TRACE;
-      return threadState == objectThreadState;
+      if (threadState == objectThreadState)
+        return true;
+
+      // we also want to remove the callback if the language hook no longer exists.
+      //   this is a belt-and-suspenders cleanup mechanism
+      return ! XBMCAddon::Python::LanguageHook::oneHasRegisteredClass(obj);
     }
   }
 }

xbmc/interfaces/python/CallbackHandler.h

@@ -44,8 +44,8 @@ namespace XBMCAddon
        *  handling callbacks in the appropriate thread.
        */
       PythonCallbackHandler();
-      virtual bool isThreadStateOk();
-      virtual bool shouldRemoveCallback(void* threadState);
+      virtual bool isStateOk(AddonClass* obj);
+      virtual bool shouldRemoveCallback(AddonClass* obj, void* threadState);
     };
   }
 }

xbmc/interfaces/python/LanguageHook.cpp

@@ -21,6 +21,7 @@
 
 
 #include "LanguageHook.h"
+#include "CallbackHandler.h"
 #include "XBPython.h"
 
 #include "interfaces/legacy/AddonUtils.h"
@@ -33,15 +34,19 @@ namespace XBMCAddon
   {
     static AddonClass::Ref<LanguageHook> instance;
 
-    static CCriticalSection ccrit;
-    static bool isInited = false;
-    static xbmcutil::InitFlag flag(isInited);
+    static CCriticalSection hooksMutex;
+    std::map<PyInterpreterState*,AddonClass::Ref<LanguageHook> > LanguageHook::hooks;
 
     // vtab instantiation
-    LanguageHook::~LanguageHook() { }
+    LanguageHook::~LanguageHook()
+    {
+      TRACE;
+      XBMCAddon::LanguageHook::deallocating();
+    }
 
     void LanguageHook::makePendingCalls()
     {
+      TRACE;
       PythonCallbackHandler::makePendingCalls();
     }
 
@@ -57,21 +62,37 @@ namespace XBMCAddon
       PyGILLock::acquireGil();
     }
 
-    LanguageHook* LanguageHook::getInstance() 
+    void LanguageHook::registerMe()
     {
-      if (!isInited) // in this case we're being called from a static initializer
-      {
-        if (instance.isNull())
-          instance = new LanguageHook();
-      }
-      else
+      TRACE;
+      CSingleLock lock(hooksMutex);
+      hooks[m_interp] = AddonClass::Ref<LanguageHook>(this);
+    }
+
+    void LanguageHook::unregisterMe()
+    {
+      TRACE;
+      CSingleLock lock(hooksMutex);
+      hooks.erase(m_interp);
+    }
+
+    AddonClass::Ref<LanguageHook> LanguageHook::getIfExists(PyInterpreterState* interp)
+    {
+      TRACE;
+      CSingleLock lock(hooksMutex);
+      std::map<PyInterpreterState*,AddonClass::Ref<LanguageHook> >::iterator iter = hooks.find(interp);
+      return iter == hooks.end() ? AddonClass::Ref<LanguageHook>(NULL) : AddonClass::Ref<LanguageHook>(iter->second);
+    }
+
+    bool LanguageHook::oneHasRegisteredClass(AddonClass* obj)
+    {
+      for (std::map<PyInterpreterState*,AddonClass::Ref<LanguageHook> >::iterator iter = hooks.begin();
+           iter != hooks.end(); iter++)
       {
-        CSingleLock lock (ccrit);
-        if (instance.isNull())
-          instance = new LanguageHook();
+        if ((iter->second)->hasRegisteredClass(obj))
+          return true;
       }
-
-      return instance.get();
+      return false;
     }
 
     /**
@@ -88,11 +109,13 @@ namespace XBMCAddon
      */
     XBMCAddon::CallbackHandler* LanguageHook::getCallbackHandler()
     { 
+      TRACE;
       return new PythonCallbackHandler();
     }
 
     String LanguageHook::getAddonId()
     {
+      TRACE;
       const char* id = NULL;
 
       // Get a reference to the main module
@@ -108,6 +131,7 @@ namespace XBMCAddon
 
     String LanguageHook::getAddonVersion()
     {
+      TRACE;
       // Get a reference to the main module
       // and global dictionary
       PyObject* main_module = PyImport_AddModule((char*)"__main__");
@@ -119,14 +143,36 @@ namespace XBMCAddon
       return version;
     }
 
-    void LanguageHook::registerPlayerCallback(IPlayerCallback* player) { g_pythonParser.RegisterPythonPlayerCallBack(player); }
-    void LanguageHook::unregisterPlayerCallback(IPlayerCallback* player) { g_pythonParser.UnregisterPythonPlayerCallBack(player); }
-    void LanguageHook::registerMonitorCallback(XBMCAddon::xbmc::Monitor* monitor) { g_pythonParser.RegisterPythonMonitorCallBack(monitor); }
-    void LanguageHook::unregisterMonitorCallback(XBMCAddon::xbmc::Monitor* monitor) { g_pythonParser.UnregisterPythonMonitorCallBack(monitor); }
+    void LanguageHook::registerPlayerCallback(IPlayerCallback* player) { TRACE; g_pythonParser.RegisterPythonPlayerCallBack(player); }
+    void LanguageHook::unregisterPlayerCallback(IPlayerCallback* player) { TRACE; g_pythonParser.UnregisterPythonPlayerCallBack(player); }
+    void LanguageHook::registerMonitorCallback(XBMCAddon::xbmc::Monitor* monitor) { TRACE; g_pythonParser.RegisterPythonMonitorCallBack(monitor); }
+    void LanguageHook::unregisterMonitorCallback(XBMCAddon::xbmc::Monitor* monitor) { TRACE; g_pythonParser.UnregisterPythonMonitorCallBack(monitor); }
 
     bool LanguageHook::waitForEvent(CEvent& hEvent, unsigned int milliseconds)
     { 
+      TRACE;
       return g_pythonParser.WaitForEvent(hEvent,milliseconds);
     }
+
+    void LanguageHook::registerAddonClass(AddonClass* obj)
+    {
+      TRACE;
+      Synchronize l(*this);
+      currentObjects.insert(obj);
+    }
+
+    void LanguageHook::unregisterAddonClass(AddonClass* obj)
+    {
+      TRACE;
+      Synchronize l(*this);
+      currentObjects.erase(obj);
+    }
+
+    bool LanguageHook::hasRegisteredClass(AddonClass* obj)
+    {
+      TRACE;
+      Synchronize l(*this);
+      return currentObjects.find(obj) != currentObjects.end();
+    }
   }
 }

xbmc/interfaces/python/LanguageHook.h

@@ -28,10 +28,12 @@
 #include <Python.h>
 
 #include "interfaces/legacy/LanguageHook.h"
-#include "interfaces/python/CallbackHandler.h"
 #include "threads/ThreadLocal.h"
 #include "threads/Event.h"
 
+#include <set>
+#include <map>
+
 namespace XBMCAddon
 {
   namespace Python
@@ -45,10 +47,17 @@ namespace XBMCAddon
      */
     class LanguageHook : public XBMCAddon::LanguageHook
     {
-      LanguageHook() : XBMCAddon::LanguageHook("Python::LanguageHook")  {  }
+      PyInterpreterState* m_interp;
+      CCriticalSection crit;
+      std::set<AddonClass*> currentObjects;
+
+      static std::map<PyInterpreterState*,AddonClass::Ref<LanguageHook> > hooks;
 
     public:
 
+      inline LanguageHook(PyInterpreterState* interp) : 
+        XBMCAddon::LanguageHook("Python::LanguageHook"), m_interp(interp)  {  }
+
       virtual ~LanguageHook();
 
       virtual void delayedCallOpen();
@@ -78,7 +87,16 @@ namespace XBMCAddon
       virtual void unregisterMonitorCallback(XBMCAddon::xbmc::Monitor* monitor);
       virtual bool waitForEvent(CEvent& hEvent, unsigned int milliseconds);
 
-      static LanguageHook* getInstance();
+      static AddonClass::Ref<LanguageHook> getIfExists(PyInterpreterState* interp);
+      static bool oneHasRegisteredClass(AddonClass* obj);
+
+      void registerAddonClass(AddonClass* obj);
+      void unregisterAddonClass(AddonClass* obj);
+      bool hasRegisteredClass(AddonClass* obj);
+      inline bool hasRegisteredClasses() { Synchronize l(*this); return currentObjects.size() > 0; }
+
+      void unregisterMe();
+      void registerMe();
     };
   }
 }

xbmc/interfaces/python/PythonSwig.cpp.template

@@ -180,7 +180,7 @@ void doMethod(method, MethodType methodType)
     }
     // now do the method call itself
     if (!destructor) {
-    if (constructor || !clazz) {  %>      XBMCAddon::SetLanguageHookGuard slhg(XBMCAddon::Python::LanguageHook::getInstance());<% println() }
+    if (constructor || !clazz) {  %>      XBMCAddon::SetLanguageHookGuard slhg(XBMCAddon::Python::LanguageHook::getIfExists(PyThreadState_Get()->interp).get());<% println() }
 %>      <%
     if (returns != "void") { %>apiResult = (${SwigTypeParser.SwigType_lstr(returns)})<% }
     if (clazz && !constructor) {

xbmc/interfaces/python/XBPyThread.cpp

@@ -158,6 +158,9 @@ void XBPyThread::Process()
   // swap in my thread state
   PyThreadState_Swap(state);
 
+  XBMCAddon::AddonClass::Ref<XBMCAddon::Python::LanguageHook> languageHook(new XBMCAddon::Python::LanguageHook(state->interp));
+  languageHook->registerMe();
+
   m_pExecuter->InitializeInterpreter(addon);
 
   CLog::Log(LOGDEBUG, "%s - The source file to load is %s", __FUNCTION__, m_source);
@@ -369,8 +372,26 @@ void XBPyThread::Process()
   m_pExecuter->DeInitializeInterpreter();
 
   Py_EndInterpreter(state);
-  PyThreadState_Swap(NULL);
 
+  // This is a total hack. Python doesn't necessarily release
+  // all of the objects associated with the interpreter when
+  // you end the interpreter. As a result there are objects 
+  // managed by the windowing system that still receive events
+  // until python decides to clean them up. Python will eventually
+  // clean them up on the creation or ending of a subsequent
+  // interpreter. So we are going to keep creating and ending
+  // interpreters until we have no more python objects hanging
+  // around.
+  while (languageHook->hasRegisteredClasses())
+  {
+    PyThreadState* tmpstate = Py_NewInterpreter();
+    Py_EndInterpreter(tmpstate);
+  }
+
+  // unregister the language hook prior to ending the interpreter
+  languageHook->unregisterMe();
+
+  PyThreadState_Swap(NULL);
   PyEval_ReleaseLock();
 }
 

xbmc/interfaces/python/swig.cpp

@@ -202,7 +202,8 @@ namespace PythonBindings
                               const char* methodNamespacePrefix, const char* methodNameForErrorString) throw (XBMCAddon::WrongTypeException)
   {
     if (pythonType == NULL || pythonType->magicNumber != XBMC_PYTHON_TYPE_MAGIC_NUMBER)
-      throw XBMCAddon::WrongTypeException("Non api type passed in place of the expected type \"%s.\"",expectedType);
+      throw XBMCAddon::WrongTypeException("Non api type passed to \"%s\" in place of the expected type \"%s.\"",
+                                          methodNameForErrorString, expectedType);
     if (!isParameterRightType(typeInfo->swigType,expectedType,methodNamespacePrefix))
     {
       // maybe it's a child class

xbmc/interfaces/python/swig.h

@@ -28,6 +28,7 @@
 #include "interfaces/legacy/AddonClass.h"
 #include "interfaces/legacy/Window.h"
 #include "threads/ThreadLocal.h"
+#include "LanguageHook.h"
 
 namespace PythonBindings
 {
@@ -104,14 +105,44 @@ namespace PythonBindings
       doretrieveApiInstance(((PyHolder*)pythonType),((PyHolder*)pythonType)->typeInfo, expectedType, methodNamespacePrefix, methodNameForErrorString);
   }
 
-  inline void prepareForReturn(XBMCAddon::AddonClass* c) { if(c) c->Acquire(); }
+  inline void prepareForReturn(XBMCAddon::AddonClass* c) 
+  {
+    TRACE;
+    if(c) { 
+      c->Acquire(); 
+      PyThreadState* state = PyThreadState_Get();
+      XBMCAddon::Python::LanguageHook::getIfExists(state->interp)->registerAddonClass(c);
+    }
+  }
 
-  inline void cleanForDealloc(XBMCAddon::AddonClass* c) { if(c) c->Release(); }
+  inline void cleanForDealloc(XBMCAddon::AddonClass* c) 
+  { 
+    TRACE;
+    if(c){
+      PyThreadState* state = PyThreadState_Get();
+      XBMCAddon::AddonClass::Ref<XBMCAddon::Python::LanguageHook> lh = 
+        XBMCAddon::Python::LanguageHook::getIfExists(state->interp);
+      if (lh.isNotNull()) lh->unregisterAddonClass(c);
+      c->Release();
+    }
+  }
 
   /**
    * There is a Catch-22 in the destruction of a Window. This resolves that.
    */
-  inline void cleanForDealloc(XBMCAddon::xbmcgui::Window* c) { if(c) { c->dispose(); c->Release(); } }
+  inline void cleanForDealloc(XBMCAddon::xbmcgui::Window* c) 
+  {
+    TRACE;
+    if(c) 
+    { 
+      PyThreadState* state = PyThreadState_Get();
+      XBMCAddon::AddonClass::Ref<XBMCAddon::Python::LanguageHook> lh = 
+        XBMCAddon::Python::LanguageHook::getIfExists(state->interp);
+      if (lh.isNotNull()) lh->unregisterAddonClass(c);
+      c->dispose();
+      c->Release(); 
+    } 
+  }
 
   /**
    * This method allows for conversion of the native api Type to the Python type