Permalink
Browse files

[fix] When an XBMCAddon::xbmcgui::Window class was deleted, it would …

…occationally still be required to handle calls from the CGUIWindow system but by then it's deleted. This resolves that by reference counting Window classes from both Python AND the CGUIWindow system so that it's not finally deleted until both are done with it. In this case the 'unregister' work that was previously done in the 'deallocating' method needed to be moved to a separate method (now called 'dispose') and invoked when python is finished with the Window rather than on destruction.
  • Loading branch information...
1 parent 3d20e6d commit 0115e2ba913bbf63434ae79ce7c4aa07083fe4af Jim Carroll committed Nov 11, 2012
@@ -62,7 +62,7 @@ namespace XBMCAddon
CGUIWindow* ProxyExistingWindowInterceptor::get() { TRACE; return cguiwindow; }
Window::Window(const char* classname) throw (WindowException):
- AddonCallback(classname), window(NULL), iWindowId(-1),
+ AddonCallback(classname), isDisposed(false), window(NULL), iWindowId(-1),
iOldWindowId(0), iCurrentControlId(3000), bModal(false), m_actionEvent(true),
canPulse(true), existingWindow(false), destroyAfterDeInit(false)
{
@@ -74,7 +74,7 @@ namespace XBMCAddon
* This just creates a default window.
*/
Window::Window(int existingWindowId) throw (WindowException) :
- AddonCallback("Window"), window(NULL), iWindowId(-1),
+ AddonCallback("Window"), isDisposed(false), window(NULL), iWindowId(-1),
iOldWindowId(0), iCurrentControlId(3000), bModal(false), m_actionEvent(true),
canPulse(false), existingWindow(true), destroyAfterDeInit(false)
{
@@ -110,14 +110,19 @@ namespace XBMCAddon
void Window::deallocating()
{
- TRACE;
-
AddonCallback::deallocating();
- // if !window then we've been here already
- if (window)
+ dispose();
+ }
+
+ void Window::dispose()
+ {
+ TRACE;
+
+ CSingleLock lock(g_graphicsContext);
+ if (!isDisposed)
{
- CSingleLock lock(g_graphicsContext);
+ isDisposed = true;
// no callbacks are possible any longer
// - this will be handled by the parent constructor
@@ -174,9 +179,6 @@ namespace XBMCAddon
}
vecControls.clear();
-
- window->clear();
- window = NULL;
}
}
@@ -77,6 +77,7 @@ namespace XBMCAddon
class Window : public AddonCallback
{
friend class WindowDialogMixin;
+ bool isDisposed;
protected:
#ifndef SWIG
InterceptorBase* window;
@@ -126,7 +127,6 @@ namespace XBMCAddon
SWIGHIDDENVIRTUAL void PulseActionEvent();
SWIGHIDDENVIRTUAL void WaitForActionEvent();
-
#endif
public:
@@ -144,6 +144,11 @@ namespace XBMCAddon
SWIGHIDDENVIRTUAL bool IsDialog() const { TRACE; return false; };
SWIGHIDDENVIRTUAL bool IsModalDialog() const { TRACE; return false; };
SWIGHIDDENVIRTUAL bool IsMediaWindow() const { TRACE; return false; };
+ SWIGHIDDENVIRTUAL void dispose();
+
+ // This is called from the InterceptorBase destructor to prevent further
+ // use of the interceptor from the window.
+ inline void interceptorClear() { Synchronize lock(*this); window = NULL; }
#endif
// callback takes a parameter
@@ -42,7 +42,7 @@ namespace XBMCAddon
class InterceptorBase
{
protected:
- Window* window;
+ AddonClass::Ref<Window> window;
XbmcThreads::ThreadLocal<ref> upcallTls;
InterceptorBase() : window(NULL) { upcallTls.set(NULL); }
@@ -63,9 +63,7 @@ namespace XBMCAddon
bool up() { bool ret = (upcallTls.get() != NULL); upcallTls.set(NULL); return ret; }
public:
- // TODO: Add thread safety
- void clear() { window = NULL; }
- virtual ~InterceptorBase() {}
+ virtual ~InterceptorBase() { if (window.isNotNull()) { window->interceptorClear(); } }
virtual CGUIWindow* get() = 0;
@@ -111,8 +109,8 @@ namespace XBMCAddon
* overloaded -> operator.
*/
-#define checkedb(methcall) ( window ? window-> methcall : false )
-#define checkedv(methcall) { if (window) window-> methcall ; }
+#define checkedb(methcall) ( window.isNotNull() ? window-> methcall : false )
+#define checkedv(methcall) { if (window.isNotNull()) window-> methcall ; }
template <class P /* extends CGUIWindow*/> class Interceptor :
public P, public InterceptorBase
@@ -133,7 +131,7 @@ namespace XBMCAddon
XBMCAddonUtils::TraceGuard tg;
CLog::Log(LOGDEBUG, "%sNEWADDON constructing %s 0x%lx", tg.getSpaces(),classname.c_str(), (long)(((void*)this)));
#endif
- window = _window;
+ window.reset(_window);
P::SetLoadType(CGUIWindow::LOAD_ON_GUI_INIT);
}
@@ -146,7 +144,7 @@ namespace XBMCAddon
XBMCAddonUtils::TraceGuard tg;
CLog::Log(LOGDEBUG, "%sNEWADDON constructing %s 0x%lx", tg.getSpaces(),classname.c_str(), (long)(((void*)this)));
#endif
- window = _window;
+ window.reset(_window);
P::SetLoadType(CGUIWindow::LOAD_ON_GUI_INIT);
}
@@ -48,8 +48,8 @@ namespace XBMCAddon
* add behavior for a few more virtual functions that were unneccessary
* in the Window or WindowDialog.
*/
-#define checkedb(methcall) ( window ? xwin-> methcall : false )
-#define checkedv(methcall) { if (window) xwin-> methcall ; }
+#define checkedb(methcall) ( window.isNotNull() ? xwin-> methcall : false )
+#define checkedv(methcall) { if (window.isNotNull()) xwin-> methcall ; }
// TODO: This should be done with template specialization
@@ -26,6 +26,7 @@
#include "utils/StdString.h"
#include "interfaces/legacy/Exception.h"
#include "interfaces/legacy/AddonClass.h"
+#include "interfaces/legacy/Window.h"
#include "threads/ThreadLocal.h"
namespace PythonBindings
@@ -106,6 +107,11 @@ namespace PythonBindings
inline void cleanForDealloc(XBMCAddon::AddonClass* c) { if(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(); } }
+
/**
* This method allows for conversion of the native api Type to the Python type
*

0 comments on commit 0115e2b

Please sign in to comment.