Permalink
Browse files

Several stability improvements including reference counting the Addon…

…Classes from the LanguageHook, more persistent attempt to free python objects by using the gc explicitly and also better tracking and logging of python objects that appear to leak.
  • Loading branch information...
1 parent 8a0aa42 commit fc1891f50312e3761337813294fbd3fe95e5a9b4 Jim Carroll committed Dec 16, 2012
@@ -1183,12 +1183,17 @@ namespace XBMCAddon
void ControlList::addItemStream(const String& fileOrUrl, bool sendMessage) throw(UnimplementedException,WindowException)
{
- addListItem(ListItem::fromString(fileOrUrl),sendMessage);
+ internAddListItem(ListItem::fromString(fileOrUrl),sendMessage);
}
void ControlList::addListItem(const XBMCAddon::xbmcgui::ListItem* pListItem, bool sendMessage) throw(UnimplementedException,WindowException)
{
- if (pListItem == NULL)
+ internAddListItem(pListItem,sendMessage);
+ }
+
+ void ControlList::internAddListItem(AddonClass::Ref<ListItem> pListItem, bool sendMessage) throw (WindowException)
+ {
+ if (pListItem.isNull())
throw WindowException("NULL ListItem passed to ControlList::addListItem");
// add item to objects vector
@@ -621,6 +621,8 @@ namespace XBMCAddon
*/
class ControlList : public Control
{
+ void internAddListItem(AddonClass::Ref<ListItem> listitem, bool sendMessage) throw(WindowException);
+
public:
ControlList(long x, long y, long width, long height, const char* font = NULL,
const char* textColor = NULL, const char* buttonTexture = NULL,
@@ -57,16 +57,16 @@ namespace XBMCAddon
#ifndef SWIG
inline ListItem(CFileItemPtr pitem) : AddonClass("ListItem"), item(pitem) {}
-#endif
-
- virtual ~ListItem();
- static inline ListItem* fromString(const String& str)
+ static inline AddonClass::Ref<ListItem> fromString(const String& str)
{
- ListItem* ret = new ListItem();
+ AddonClass::Ref<ListItem> ret = AddonClass::Ref<ListItem>(new ListItem());
ret->item.reset(new CFileItem(str));
return ret;
}
+#endif
+
+ virtual ~ListItem();
/**
* getLabel() -- Returns the listitem label.
@@ -158,14 +158,16 @@ namespace XBMCAddon
{
TRACE;
Synchronize l(*this);
+ obj->Acquire();
currentObjects.insert(obj);
}
void LanguageHook::UnregisterAddonClassInstance(AddonClass* obj)
{
TRACE;
Synchronize l(*this);
- currentObjects.erase(obj);
+ if (currentObjects.erase(obj) > 0)
+ obj->Release();
}
bool LanguageHook::HasRegisteredAddonClassInstance(AddonClass* obj)
@@ -112,6 +112,7 @@ void XBPyThread::setSource(const CStdString &src)
m_source = new char[strsrc.GetLength()+1];
strcpy(m_source, strsrc);
#else
+ if (m_source) delete [] m_source;
m_source = new char[src.GetLength()+1];
strcpy(m_source, src);
#endif
@@ -145,6 +146,27 @@ int XBPyThread::setArgv(const std::vector<CStdString> &argv)
return 0;
}
+#define GC_SCRIPT \
+ "import gc\n" \
+ "gc.collect(2)\n"
+
+static const CStdString getListOfAddonClassesAsString(XBMCAddon::AddonClass::Ref<XBMCAddon::Python::LanguageHook>& languageHook)
+{
+ CStdString message;
+ XBMCAddon::AddonClass::Synchronize l(*(languageHook.get()));
+ std::set<XBMCAddon::AddonClass*>& acs = languageHook->GetRegisteredAddonClasses();
+ bool firstTime = true;
+ for (std::set<XBMCAddon::AddonClass*>::iterator iter = acs.begin();
+ iter != acs.end(); iter++)
+ {
+ if (!firstTime) message += ",";
+ else firstTime = false;
+ message += (*iter)->GetClassname().c_str();
+ }
+
+ return message;
+}
+
void XBPyThread::Process()
{
CLog::Log(LOGDEBUG,"Python thread: start processing");
@@ -376,6 +398,10 @@ void XBPyThread::Process()
m_pExecuter->DeInitializeInterpreter();
+ // run the gc before finishing
+ if (!m_stopping && languageHook->HasRegisteredAddonClasses() && PyRun_SimpleString(GC_SCRIPT) == -1)
+ CLog::Log(LOGERROR,"Failed to run the gc to clean up after running prior to shutting down the Interpreter %s",m_source);
+
Py_EndInterpreter(state);
// This is a total hack. Python doesn't necessarily release
@@ -387,40 +413,34 @@ void XBPyThread::Process()
// interpreter. So we are going to keep creating and ending
// interpreters until we have no more python objects hanging
// around.
- int countLimit;
- for (countLimit = 0; languageHook->HasRegisteredAddonClasses() && countLimit < 10; countLimit++)
- {
- PyThreadState* tmpstate = Py_NewInterpreter();
- Py_EndInterpreter(tmpstate);
- }
-
- // If necessary and successfull, debug log the results.
- if (countLimit > 0 && !languageHook->HasRegisteredAddonClasses())
- CLog::Log(LOGDEBUG,"It took %d Py_NewInterpreter/Py_EndInterpreter calls"
- " to clean up the classes leftover from running \"%s.\"",
- countLimit,m_source);
-
- // If not successful, produce an error message detailing what's been left behind
if (languageHook->HasRegisteredAddonClasses())
{
- CStdString message;
- message.Format("The python script \"%s\" has left several "
- "classes in memory that should have been cleaned up. The classes include: ",
- m_source);
-
- { XBMCAddon::AddonClass::Synchronize l(*(languageHook.get()));
- std::set<XBMCAddon::AddonClass*>& acs = languageHook->GetRegisteredAddonClasses();
- bool firstTime = true;
- for (std::set<XBMCAddon::AddonClass*>::iterator iter = acs.begin();
- iter != acs.end(); iter++)
- {
- if (!firstTime) message += ",";
- else firstTime = false;
- message += (*iter)->GetClassname().c_str();
- }
+ CLog::Log(LOGDEBUG, "The python script \"%s\" has left several "
+ "classes in memory that we will be attempting to clean up. The classes include: %s",
+ m_source, getListOfAddonClassesAsString(languageHook).c_str());
+
+ int countLimit;
+ for (countLimit = 0; languageHook->HasRegisteredAddonClasses() && countLimit < 100; countLimit++)
+ {
+ PyThreadState* tmpstate = Py_NewInterpreter();
+ PyThreadState* oldstate = PyThreadState_Swap(tmpstate);
+ if (PyRun_SimpleString(GC_SCRIPT) == -1)
+ CLog::Log(LOGERROR,"Failed to run the gc to clean up after running %s",m_source);
+ PyThreadState_Swap(oldstate);
+ Py_EndInterpreter(tmpstate);
}
-
- CLog::Log(LOGERROR, "%s", message.c_str());
+
+ // If necessary and successfull, debug log the results.
+ if (countLimit > 0 && !languageHook->HasRegisteredAddonClasses())
+ CLog::Log(LOGDEBUG,"It took %d Py_NewInterpreter/Py_EndInterpreter calls"
+ " to clean up the classes leftover from running \"%s.\"",
+ countLimit,m_source);
+
+ // If not successful, produce an error message detailing what's been left behind
+ if (languageHook->HasRegisteredAddonClasses())
+ CLog::Log(LOGERROR, "The python script \"%s\" has left several "
+ "classes in memory that we couldn't clean up. The classes include: %s",
+ m_source, getListOfAddonClassesAsString(languageHook).c_str());
}
// unregister the language hook
@@ -624,7 +624,7 @@ void XBPython::Finalize()
// Other methods that rely on this flag from an incorrect interpretation.
m_bInitialized = false;
PyThreadState* curTs = (PyThreadState*)m_mainThreadState;
- m_mainThreadState = NULL;
+ m_mainThreadState = NULL; // clear the main thread state before releasing the lock
{
CSingleExit exit(m_critSection);
PyEval_AcquireLock();
@@ -235,12 +235,23 @@ namespace PythonBindings
static bool handleInterpRegistrationForClean(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->UnregisterAddonClassInstance(c);
- return true;
+ XBMCAddon::AddonClass::Ref<XBMCAddon::AddonClass>(c->GetLanguageHook());
+
+ if (lh.isNotNull())
+ {
+ lh->UnregisterAddonClassInstance(c);
+ return true;
+ }
+ else
+ {
+ PyThreadState* state = PyThreadState_Get();
+ lh = XBMCAddon::Python::LanguageHook::GetIfExists(state->interp);
+ if (lh.isNotNull()) lh->UnregisterAddonClassInstance(c);
+ return true;
+ }
}
return false;
}

0 comments on commit fc1891f

Please sign in to comment.