From a2799badc68a43e09e4d4dfcd015ea1745c29906 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Fri, 20 Jan 2012 14:47:42 +0800 Subject: [PATCH] Fix SIGSEGV when a window is minimized. (LP: #918329) The crash was caused by 'minimizedWindows' leaking pointers to windows that had been deleted, and then later trying to dereference those pointers. The reason for the leak appears to be a reference leak in the boost::shared_ptr's to CompizMinimizedWindowHandler. Reverting to regular pointers avoids the reference leak, avoids the crash, and makes the code easier to understand (and debug). --- .../src/compizminimizedwindowhandler.h | 54 ++++++++----------- plugins/unityshell/src/unityshell.cpp | 36 +++++-------- plugins/unityshell/src/unityshell.h | 5 +- 3 files changed, 41 insertions(+), 54 deletions(-) diff --git a/plugins/unityshell/src/compizminimizedwindowhandler.h b/plugins/unityshell/src/compizminimizedwindowhandler.h index d5533210e..8125a036f 100644 --- a/plugins/unityshell/src/compizminimizedwindowhandler.h +++ b/plugins/unityshell/src/compizminimizedwindowhandler.h @@ -63,9 +63,8 @@ class CompizMinimizedWindowHandler: static void handleEvent (XEvent *event); static std::list minimizingWindows; - typedef CompizMinimizedWindowHandler CompizMinimizedWindowHandler_complete; - typedef boost::shared_ptr Ptr; - typedef std::list List; + typedef CompizMinimizedWindowHandler Type; + typedef std::list List; protected: virtual std::vector getTransients (); @@ -74,10 +73,14 @@ class CompizMinimizedWindowHandler: PrivateCompizMinimizedWindowHandler *priv; static bool handleEvents; - static std::list minimizedWindows; + static List minimizedWindows; }; } +/* XXX minimizedWindows should be removed because it is dangerous to keep + * a list of windows separate to compiz-core. The list could get out of + * sync and cause more crashes like LP: #918329, LP: #864758. + */ template typename compiz::CompizMinimizedWindowHandler::List compiz::CompizMinimizedWindowHandler::minimizedWindows; @@ -100,12 +103,7 @@ compiz::CompizMinimizedWindowHandler::CompizMinimizedWindowHandl template compiz::CompizMinimizedWindowHandler::~CompizMinimizedWindowHandler () { - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_full; - - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (Window::get (priv->mWindow)->mMinimizeHandler); - - minimizedWindows.remove (compizMinimizeHandler); + minimizedWindows.remove (this); } template @@ -144,18 +142,13 @@ compiz::CompizMinimizedWindowHandler::minimize () Atom wmState = XInternAtom (screen->dpy (), "WM_STATE", 0); unsigned long data[2]; - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_full; - std::vector transients = getTransients (); handleEvents = true; priv->mWindow->windowNotify (CompWindowNotifyMinimize); priv->mWindow->changeState (priv->mWindow->state () | CompWindowStateHiddenMask); - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (Window::get (priv->mWindow)->mMinimizeHandler); - - minimizedWindows.push_back (compizMinimizeHandler); + minimizedWindows.push_back (this); for (unsigned int &w : transients) { @@ -163,8 +156,10 @@ compiz::CompizMinimizedWindowHandler::minimize () if (win) { - Window::get (win)->mMinimizeHandler = MinimizedWindowHandler::Ptr (new CompizMinimizedWindowHandler (win)); - Window::get (win)->mMinimizeHandler->minimize (); + Window *w = Window::get (win); + if (!w->mMinimizeHandler) + w->mMinimizeHandler = new Type (win); + w->mMinimizeHandler->minimize (); } } @@ -226,14 +221,9 @@ compiz::CompizMinimizedWindowHandler::unminimize () Atom wmState = XInternAtom (screen->dpy (), "WM_STATE", 0); unsigned long data[2]; - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_full; - std::vector transients = getTransients (); - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (Window::get (priv->mWindow)->mMinimizeHandler); - - minimizedWindows.remove (compizMinimizeHandler); + minimizedWindows.remove (this); priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), true); @@ -247,10 +237,13 @@ compiz::CompizMinimizedWindowHandler::unminimize () if (win) { - if (Window::get (win)->mMinimizeHandler) - Window::get (win)->mMinimizeHandler->unminimize (); - - Window::get (win)->mMinimizeHandler.reset (); + Window *w = Window::get (win); + if (w && w->mMinimizeHandler) + { + w->mMinimizeHandler->unminimize (); + delete w->mMinimizeHandler; + w->mMinimizeHandler = NULL; + } } } @@ -330,10 +323,9 @@ compiz::CompizMinimizedWindowHandler::handleEvent (XEvent *event if (w) { - typedef compiz::CompizMinimizedWindowHandler plugin_handler; Window *pw = Window::get (w); - plugin_handler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (pw->mMinimizeHandler); + Type *compizMinimizeHandler = pw->mMinimizeHandler; + /* Restore and re-save input shape and remove */ if (compizMinimizeHandler) { diff --git a/plugins/unityshell/src/unityshell.cpp b/plugins/unityshell/src/unityshell.cpp index f35bd4e20..168049cbb 100644 --- a/plugins/unityshell/src/unityshell.cpp +++ b/plugins/unityshell/src/unityshell.cpp @@ -1742,11 +1742,7 @@ bool UnityWindow::glPaint(const GLWindowPaintAttrib& attrib, if (mMinimizeHandler) { - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_unity; - - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (mMinimizeHandler); - mask |= compizMinimizeHandler->getPaintMask (); + mask |= mMinimizeHandler->getPaintMask (); } else if (mShowdesktopHandler) { @@ -1815,7 +1811,7 @@ UnityWindow::minimize () if (!mMinimizeHandler) { - mMinimizeHandler = compiz::MinimizedWindowHandler::Ptr (new compiz::CompizMinimizedWindowHandler (window)); + mMinimizeHandler = new UnityMinimizedHandler (window); mMinimizeHandler->minimize (); } } @@ -1826,14 +1822,15 @@ UnityWindow::unminimize () if (mMinimizeHandler) { mMinimizeHandler->unminimize (); - mMinimizeHandler.reset (); + delete mMinimizeHandler; + mMinimizeHandler = nullptr; } } bool UnityWindow::focus () { - if (!mMinimizeHandler.get ()) + if (!mMinimizeHandler) return window->focus (); if (window->overrideRedirect ()) @@ -1865,7 +1862,7 @@ UnityWindow::focus () bool UnityWindow::minimized () { - return mMinimizeHandler.get () != NULL; + return mMinimizeHandler != nullptr; } gboolean @@ -1928,16 +1925,12 @@ void UnityWindow::windowNotify(CompWindowNotify n) window->windowNotify(n); - if (mMinimizeHandler.get () != NULL) + if (mMinimizeHandler) { /* The minimize handler will short circuit the frame * region update func and ensure that the frame * does not have a region */ - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_unity; - - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (mMinimizeHandler); - compizMinimizeHandler->windowNotify (n); + mMinimizeHandler->windowNotify (n); } else if (mShowdesktopHandler) { @@ -1976,13 +1969,9 @@ void UnityWindow::updateFrameRegion(CompRegion ®ion) /* The minimize handler will short circuit the frame * region update func and ensure that the frame * does not have a region */ - typedef compiz::CompizMinimizedWindowHandler minimized_window_handler_unity; - compiz::CompizMinimizedWindowHandler::Ptr compizMinimizeHandler = - boost::dynamic_pointer_cast (mMinimizeHandler); - - if (compizMinimizeHandler) - compizMinimizeHandler->updateFrameRegion (region); + if (mMinimizeHandler) + mMinimizeHandler->updateFrameRegion (region); else if (mShowdesktopHandler) mShowdesktopHandler->updateFrameRegion (region); else @@ -2453,6 +2442,7 @@ UnityWindow::UnityWindow(CompWindow* window) , PluginClassHandler(window) , window(window) , gWindow(GLWindow::get(window)) + , mMinimizeHandler(nullptr) , mShowdesktopHandler(nullptr) , focusdesktop_handle_(0) { @@ -2519,8 +2509,10 @@ UnityWindow::~UnityWindow() window->minimizedSetEnabled (this, false); window->minimize (); - mMinimizeHandler.reset (); + delete mMinimizeHandler; + mMinimizeHandler = nullptr; } + if (mShowdesktopHandler) delete mShowdesktopHandler; diff --git a/plugins/unityshell/src/unityshell.h b/plugins/unityshell/src/unityshell.h index 8e9608cc8..d81b1de29 100644 --- a/plugins/unityshell/src/unityshell.h +++ b/plugins/unityshell/src/unityshell.h @@ -376,7 +376,10 @@ class UnityWindow : void leaveShowDesktop (); bool handleAnimations (unsigned int ms); - compiz::MinimizedWindowHandler::Ptr mMinimizeHandler; + typedef compiz::CompizMinimizedWindowHandler + UnityMinimizedHandler; + UnityMinimizedHandler *mMinimizeHandler; + UnityShowdesktopHandler *mShowdesktopHandler; private: