Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Stability fixes #1947

Merged
merged 4 commits into from

1 participant

@jimfcarroll
Collaborator

Some stability fixes

Jim Carroll added some commits
Jim Carroll Potential finalize fix. 85789f1
Jim Carroll [cosmetic] Added a couple additional TRACE points. b3c5667
Jim Carroll Added std::set semantics to the AddonClass::Ref. 8a0aa42
Jim Carroll 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.
fc1891f
@jimfcarroll jimfcarroll merged commit 7eec46e into xbmc:master
@jimfcarroll jimfcarroll deleted the jimfcarroll:stability-fixes branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 16, 2012
  1. Potential finalize fix.

    Jim Carroll authored
  2. [cosmetic] Added a couple additional TRACE points.

    Jim Carroll authored
  3. Added std::set semantics to the AddonClass::Ref.

    Jim Carroll authored
  4. Several stability improvements including reference counting the Addon…

    Jim Carroll authored
    …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.
This page is out of date. Refresh to see the latest.
View
2  xbmc/interfaces/legacy/AddonClass.h
@@ -199,6 +199,8 @@ namespace XBMCAddon
inline bool isNotNull() const { refcheck; return ac != NULL; }
inline bool isSet() const { refcheck; return ac != NULL; }
inline bool operator!() const { refcheck; return ac == NULL; }
+ inline bool operator==(const AddonClass::Ref<T>& oref) const { refcheck; return ac == oref.ac; }
+ inline bool operator<(const AddonClass::Ref<T>& oref) const { refcheck; return ac < oref.ac; } // std::set semantics
// This is there only for boost compatibility
template<class O> inline void reset(Ref<O> const & oref) { refcheck; (*this) = static_cast<T*>(oref.get()); refcheck; }
View
2  xbmc/interfaces/legacy/CallbackHandler.cpp
@@ -33,7 +33,7 @@ namespace XBMCAddon
AddonClass::Ref<Callback> cb;
RetardedAsynchCallbackHandler* handler;
AsynchCallbackMessage(Callback* _cb, RetardedAsynchCallbackHandler* _handler) :
- AddonClass("AsynchCallbackMessage"), cb(_cb), handler(_handler) {}
+ AddonClass("AsynchCallbackMessage"), cb(_cb), handler(_handler) { TRACE; }
};
//********************************************************************
View
9 xbmc/interfaces/legacy/Control.cpp
@@ -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
View
2  xbmc/interfaces/legacy/Control.h
@@ -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,
View
10 xbmc/interfaces/legacy/ListItem.h
@@ -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.
View
4 xbmc/interfaces/python/LanguageHook.cpp
@@ -158,6 +158,7 @@ namespace XBMCAddon
{
TRACE;
Synchronize l(*this);
+ obj->Acquire();
currentObjects.insert(obj);
}
@@ -165,7 +166,8 @@ namespace XBMCAddon
{
TRACE;
Synchronize l(*this);
- currentObjects.erase(obj);
+ if (currentObjects.erase(obj) > 0)
+ obj->Release();
}
bool LanguageHook::HasRegisteredAddonClassInstance(AddonClass* obj)
View
82 xbmc/interfaces/python/XBPyThread.cpp
@@ -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
View
34 xbmc/interfaces/python/XBPython.cpp
@@ -43,6 +43,7 @@
#include "interfaces/AnnouncementManager.h"
#include "interfaces/legacy/Monitor.h"
+#include "interfaces/legacy/AddonUtils.h"
using namespace ANNOUNCEMENT;
@@ -74,12 +75,14 @@ XBPython::XBPython()
XBPython::~XBPython()
{
+ TRACE;
CAnnouncementManager::RemoveAnnouncer(this);
}
// message all registered callbacks that xbmc stopped playing
void XBPython::OnPlayBackEnded()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -116,6 +119,7 @@ void XBPython::Announce(AnnouncementFlag flag, const char *sender, const char *m
// message all registered callbacks that we started playing
void XBPython::OnPlayBackStarted()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -146,6 +150,7 @@ void XBPython::OnPlayBackPaused()
// message all registered callbacks that we resumed playing
void XBPython::OnPlayBackResumed()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -161,6 +166,7 @@ void XBPython::OnPlayBackResumed()
// message all registered callbacks that user stopped playing
void XBPython::OnPlayBackStopped()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -176,6 +182,7 @@ void XBPython::OnPlayBackStopped()
// message all registered callbacks that playback speed changed (FF/RW)
void XBPython::OnPlayBackSpeedChanged(int iSpeed)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -191,6 +198,7 @@ void XBPython::OnPlayBackSpeedChanged(int iSpeed)
// message all registered callbacks that player is seeking
void XBPython::OnPlayBackSeek(int iTime, int seekOffset)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -206,6 +214,7 @@ void XBPython::OnPlayBackSeek(int iTime, int seekOffset)
// message all registered callbacks that player chapter seeked
void XBPython::OnPlayBackSeekChapter(int iChapter)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -221,6 +230,7 @@ void XBPython::OnPlayBackSeekChapter(int iChapter)
// message all registered callbacks that next item has been queued
void XBPython::OnQueueNextItem()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -235,12 +245,14 @@ void XBPython::OnQueueNextItem()
void XBPython::RegisterPythonPlayerCallBack(IPlayerCallback* pCallback)
{
+ TRACE;
CSingleLock lock(m_critSection);
m_vecPlayerCallbackList.push_back(pCallback);
}
void XBPython::UnregisterPythonPlayerCallBack(IPlayerCallback* pCallback)
{
+ TRACE;
CSingleLock lock(m_critSection);
PlayerCallbackList::iterator it = m_vecPlayerCallbackList.begin();
while (it != m_vecPlayerCallbackList.end())
@@ -254,12 +266,14 @@ void XBPython::UnregisterPythonPlayerCallBack(IPlayerCallback* pCallback)
void XBPython::RegisterPythonMonitorCallBack(XBMCAddon::xbmc::Monitor* pCallback)
{
+ TRACE;
CSingleLock lock(m_critSection);
m_vecMonitorCallbackList.push_back(pCallback);
}
void XBPython::UnregisterPythonMonitorCallBack(XBMCAddon::xbmc::Monitor* pCallback)
{
+ TRACE;
CSingleLock lock(m_critSection);
MonitorCallbackList::iterator it = m_vecMonitorCallbackList.begin();
while (it != m_vecMonitorCallbackList.end())
@@ -273,6 +287,7 @@ void XBPython::UnregisterPythonMonitorCallBack(XBMCAddon::xbmc::Monitor* pCallba
void XBPython::OnSettingsChanged(const CStdString &ID)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -288,6 +303,7 @@ void XBPython::OnSettingsChanged(const CStdString &ID)
void XBPython::OnScreensaverActivated()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -302,6 +318,7 @@ void XBPython::OnScreensaverActivated()
void XBPython::OnScreensaverDeactivated()
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -316,6 +333,7 @@ void XBPython::OnScreensaverDeactivated()
void XBPython::OnDatabaseUpdated(const std::string &database)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -330,6 +348,7 @@ void XBPython::OnDatabaseUpdated(const std::string &database)
void XBPython::OnAbortRequested(const CStdString &ID)
{
+ TRACE;
CSingleLock lock(m_critSection);
if (m_bInitialized)
{
@@ -461,6 +480,7 @@ void XBPython::UnloadExtensionLibs()
void XBPython::InitializeInterpreter(ADDON::AddonPtr addon)
{
+ TRACE;
{
GilSafeSingleLock lock(m_critSection);
initModule_xbmcgui();
@@ -483,6 +503,7 @@ void XBPython::InitializeInterpreter(ADDON::AddonPtr addon)
void XBPython::DeInitializeInterpreter()
{
+ TRACE;
}
/**
@@ -490,6 +511,7 @@ void XBPython::DeInitializeInterpreter()
*/
void XBPython::Initialize()
{
+ TRACE;
CLog::Log(LOGINFO, "initializing python engine. ");
CSingleLock lock(m_critSection);
m_iDllScriptCounter++;
@@ -582,6 +604,7 @@ void XBPython::Initialize()
*/
void XBPython::FinalizeScript()
{
+ TRACE;
CSingleLock lock(m_critSection);
// for linux - we never release the library. its loaded and stays in memory.
if (m_iDllScriptCounter)
@@ -592,14 +615,20 @@ void XBPython::FinalizeScript()
}
void XBPython::Finalize()
{
+ TRACE;
if (m_bInitialized)
{
CLog::Log(LOGINFO, "Python, unloading python shared library because no scripts are running anymore");
+ // set the m_bInitialized flag before releasing the lock. This will prevent
+ // Other methods that rely on this flag from an incorrect interpretation.
+ m_bInitialized = false;
+ PyThreadState* curTs = (PyThreadState*)m_mainThreadState;
+ m_mainThreadState = NULL; // clear the main thread state before releasing the lock
{
CSingleExit exit(m_critSection);
PyEval_AcquireLock();
- PyThreadState_Swap((PyThreadState*)m_mainThreadState);
+ PyThreadState_Swap(curTs);
Py_Finalize();
PyEval_ReleaseLock();
@@ -619,8 +648,6 @@ void XBPython::Finalize()
// The implementation for osx can never unload the python dylib.
DllLoaderContainer::ReleaseModule(m_pDll);
#endif
- m_mainThreadState = NULL;
- m_bInitialized = false;
}
}
@@ -684,6 +711,7 @@ void XBPython::Process()
bool XBPython::StopScript(const CStdString &path)
{
+ TRACE;
int id = getScriptId(path);
if (id != -1)
{
View
2  xbmc/interfaces/python/XBPython.h
@@ -55,6 +55,7 @@ class XBPython :
public IPlayerCallback,
public ANNOUNCEMENT::IAnnouncer
{
+ void Finalize();
public:
XBPython();
virtual ~XBPython();
@@ -79,7 +80,6 @@ class XBPython :
void OnDatabaseUpdated(const std::string &database);
void OnAbortRequested(const CStdString &ID="");
void Initialize();
- void Finalize();
void FinalizeScript();
void FreeResources();
void Process();
View
19 xbmc/interfaces/python/swig.cpp
@@ -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;
}
View
1  xbmc/interfaces/swig/AddonModuleXbmc.i
@@ -51,6 +51,7 @@ using namespace xbmc;
%feature("python:method:play") Player
{
+ TRACE;
PyObject *pObject = NULL;
PyObject *pObjectListItem = NULL;
char bWindowed = false;
Something went wrong with that request. Please try again.