Merge lp:~brandontschaefer/unity/mt-grab-handle-refrence-counting-fix-6.0 into lp:unity/6.0

[Brandon Schaefer]

Fix reference counting problem which leads to memory leaks and a crash in MT grab handles.

=== Problem ===
Reference counting problem causes Window/memory leaks which causes a map to not free XIDs causing a crash (hard to reproduce)

The problem is there is a vector mGrabHandles which stores a shard_ptr then the shared_ptr is pushed inside of a map mInputHandle. So when a window closes there is more then 1 things references each grab handle window so it never gets cleaned up.

=== Fix ===
Change the map to store weak ptrs, now when a window is closed everything gets cleaned up.

=== Test ===
Working on coming up with something, a manual test wouldn't work well here since it is hard to reproduce the crash.

[Łukasz Zemczak (sil2100)]

Ok, tested, working, not causing any regressions - but is there a known manual test to reproduce this issue?

review: Approve

[Unity Merger (unity-merger)]

No commit message specified.

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.cpp'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.cpp	2011-12-22 09:56:31 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.cpp	2012-10-13 20:30:29 +0000
@@ -40,7 +40,7 @@
 }
 
 void
-unity::MT::GrabHandleGroup::raiseHandle(const boost::shared_ptr <const unity::MT::GrabHandle> &h)
+unity::MT::GrabHandleGroup::raiseHandle(const std::shared_ptr <const unity::MT::GrabHandle> &h)
 {
   mOwner->raiseGrabHandle (h);
 }

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.h	2011-12-21 08:18:47 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-group.h	2012-10-13 20:30:29 +0000
@@ -22,9 +22,7 @@
 #include <Nux/Nux.h>
 #include <glib.h>
 #include <boost/noncopyable.hpp>
-#include <boost/shared_ptr.hpp>
-#include <boost/enable_shared_from_this.hpp>
-#include <boost/weak_ptr.hpp>
+#include <memory>
 
 #include "unity-mt-grab-handle.h"
 #include "unity-mt-texture.h"
@@ -37,12 +35,12 @@
 extern unsigned int FADE_MSEC;
 
 class GrabHandleGroup :
-  public boost::enable_shared_from_this <GrabHandleGroup>,
+  public std::enable_shared_from_this <GrabHandleGroup>,
   boost::noncopyable
 {
 public:
 
-  typedef boost::shared_ptr <GrabHandleGroup> Ptr;
+  typedef std::shared_ptr <GrabHandleGroup> Ptr;
 
   static GrabHandleGroup::Ptr create (GrabHandleWindow *owner,
                                       std::vector<TextureSize> &textures);
@@ -59,7 +57,7 @@
 
   void hide();
   void show(unsigned int handles = ~0);
-  void raiseHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &);
+  void raiseHandle (const std::shared_ptr <const unity::MT::GrabHandle> &);
 
   std::vector <TextureLayout> layout(unsigned int handles);
 

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.cpp'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.cpp	2011-12-23 07:13:19 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.cpp	2012-10-13 20:30:29 +0000
@@ -19,9 +19,9 @@
 #include "unity-mt-grab-handle-impl-factory.h"
 #include "unity-mt-grab-handle.h"
 
-boost::shared_ptr <unity::MT::GrabHandle::ImplFactory> unity::MT::GrabHandle::ImplFactory::mDefault;
+std::shared_ptr <unity::MT::GrabHandle::ImplFactory> unity::MT::GrabHandle::ImplFactory::mDefault;
 
-boost::shared_ptr <unity::MT::GrabHandle::ImplFactory>
+std::shared_ptr <unity::MT::GrabHandle::ImplFactory>
 unity::MT::GrabHandle::ImplFactory::Default()
 {
   return mDefault;

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.h	2011-12-23 07:13:19 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-impl-factory.h	2012-10-13 20:30:29 +0000
@@ -22,9 +22,7 @@
 #include <Nux/Nux.h>
 #include <glib.h>
 #include <boost/noncopyable.hpp>
-#include <boost/shared_ptr.hpp>
-#include <boost/enable_shared_from_this.hpp>
-#include <boost/weak_ptr.hpp>
+#include <memory>
 
 #include "unity-mt-grab-handle.h"
 
@@ -38,7 +36,7 @@
 
     virtual ~ImplFactory() {};
 
-    static boost::shared_ptr <ImplFactory>
+    static std::shared_ptr <ImplFactory>
     Default();
 
     static void
@@ -48,7 +46,7 @@
 
   protected:
 
-    static boost::shared_ptr <ImplFactory> mDefault;
+    static std::shared_ptr <ImplFactory> mDefault;
 
     ImplFactory() {};
 };

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-window.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-window.h	2011-12-21 08:18:47 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle-window.h	2012-10-13 20:30:29 +0000
@@ -37,7 +37,7 @@
                                   int y,
                                   unsigned int direction,
 				  unsigned int button) = 0;
-    virtual void raiseGrabHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &) = 0;
+    virtual void raiseGrabHandle (const std::shared_ptr <const unity::MT::GrabHandle> &) = 0;
 };
 
 };

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.cpp'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.cpp	2011-12-21 08:18:47 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.cpp	2012-10-13 20:30:29 +0000
@@ -55,7 +55,7 @@
 unity::MT::GrabHandle::raise () const
 {
   unity::MT::GrabHandleGroup::Ptr ghg = mOwner.lock ();
-  boost::shared_ptr <const unity::MT::GrabHandle> gh = shared_from_this ();
+  std::shared_ptr <const unity::MT::GrabHandle> gh = shared_from_this ();
   ghg->raiseHandle (gh);
 }
 
@@ -98,7 +98,7 @@
 unity::MT::GrabHandle::GrabHandle(Texture::Ptr texture,
                                   unsigned int    width,
                                   unsigned int    height,
-                                  const boost::shared_ptr <GrabHandleGroup> &owner,
+                                  const std::shared_ptr <GrabHandleGroup> &owner,
 				  unsigned int    id) :
   mOwner(owner),
   mTexture (texture),
@@ -110,7 +110,7 @@
 
 unity::MT::GrabHandle::Ptr
 unity::MT::GrabHandle::create (Texture::Ptr texture, unsigned int width, unsigned int height,
-                               const boost::shared_ptr <GrabHandleGroup> &owner,
+                               const std::shared_ptr <GrabHandleGroup> &owner,
                                unsigned int id)
 {
   unity::MT::GrabHandle::Ptr p (new unity::MT::GrabHandle (texture, width, height, owner, id));

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.h	2011-12-21 08:18:47 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handle.h	2012-10-13 20:30:29 +0000
@@ -21,10 +21,8 @@
 
 #include <Nux/Nux.h>
 #include <glib.h>
+#include <memory>
 #include <boost/noncopyable.hpp>
-#include <boost/shared_ptr.hpp>
-#include <boost/enable_shared_from_this.hpp>
-#include <boost/weak_ptr.hpp>
 
 #include "unity-mt-texture.h"
 #include "unity-mt-grab-handle-window.h"
@@ -79,17 +77,17 @@
 class GrabHandleGroup;
 
 class GrabHandle :
-  public boost::enable_shared_from_this <GrabHandle>,
+  public std::enable_shared_from_this <GrabHandle>,
   boost::noncopyable
 {
 public:
 
-  typedef boost::shared_ptr <GrabHandle> Ptr;
+  typedef std::shared_ptr <GrabHandle> Ptr;
 
   static GrabHandle::Ptr create (Texture::Ptr    texture,
                                  unsigned int    width,
                                  unsigned int    height,
-                                 const boost::shared_ptr <GrabHandleGroup> &owner,
+                                 const std::shared_ptr <GrabHandleGroup> &owner,
                                  unsigned int id);
   ~GrabHandle();
 
@@ -160,10 +158,10 @@
   GrabHandle(Texture::Ptr    texture,
              unsigned int    width,
              unsigned int    height,
-             const boost::shared_ptr <GrabHandleGroup> &owner,
+             const std::shared_ptr <GrabHandleGroup> &owner,
              unsigned int id);
 
-  boost::weak_ptr <GrabHandleGroup>      mOwner;
+  std::weak_ptr <GrabHandleGroup>      mOwner;
   Texture::Ptr                           mTexture;
   unsigned int                           mId;
   nux::Geometry                          mRect;

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.cpp'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.cpp	2012-09-13 10:56:42 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.cpp	2012-10-13 20:30:29 +0000
@@ -35,7 +35,8 @@
 unity::MT::Texture::Ptr
 unity::MT::X11TextureFactory::create ()
 {
-  return boost::shared_static_cast <unity::MT::Texture> (unity::MT::X11Texture::Ptr (new unity::MT::X11Texture (mWrap)));
+  unity::MT::Texture::Ptr tp(static_cast<unity::MT::Texture*> (new unity::MT::X11Texture(mWrap)));
+  return tp;
 }
 
 unity::MT::X11Texture::X11Texture (const GLTexture::List &t)
@@ -151,7 +152,7 @@
 }
 
 void
-UnityMTGrabHandlesWindow::raiseGrabHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &h)
+UnityMTGrabHandlesWindow::raiseGrabHandle (const std::shared_ptr <const unity::MT::GrabHandle> &h)
 {
   UnityMTGrabHandlesScreen::get (screen)->raiseHandle (h, window->frame ());
 }
@@ -202,12 +203,13 @@
 }
 
 void
-UnityMTGrabHandlesScreen::raiseHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &h,
+UnityMTGrabHandlesScreen::raiseHandle (const std::shared_ptr <const unity::MT::GrabHandle> &h,
                                        Window                                                owner)
 {
   for (const auto &pair : mInputHandles)
   {
-    if (*pair.second == *h)
+    const unity::MT::GrabHandle::Ptr gh = pair.second.lock();
+    if (*gh == *h)
     {
       unsigned int mask = CWSibling | CWStackMode;
       XWindowChanges xwc;
@@ -322,10 +324,11 @@
 
       if (it != mInputHandles.end())
       {
-	if (it->second)
-          it->second->buttonPress (event->xbutton.x_root,
-                                   event->xbutton.y_root,
-                                   event->xbutton.button);
+        const unity::MT::GrabHandle::Ptr gh = it->second.lock();
+        if (gh)
+          gh->buttonPress (event->xbutton.x_root,
+                           event->xbutton.y_root,
+                           event->xbutton.button);
       }
 
       break;
@@ -346,8 +349,9 @@
 
       if (it != mInputHandles.end())
       {
-        if (it->second)
-          it->second->reposition (0, 0, unity::MT::PositionLock);
+        const unity::MT::GrabHandle::Ptr gh = it->second.lock();
+        if (gh)
+          gh->reposition (0, 0, unity::MT::PositionLock);
       }
 
       break;
@@ -457,7 +461,7 @@
        * region */
       CompRegion reg = CompRegion(layout.second.x, layout.second.y, layout.second.width, layout.second.height);
 
-      for(GLTexture * tex : boost::shared_static_cast <unity::MT::X11Texture> (layout.first)->get ())
+      for(GLTexture * tex : static_cast<unity::MT::X11Texture*>(layout.first.get())->get())
       {
         GLTexture::MatrixList matl;
         GLTexture::Matrix     mat = tex->matrix();
@@ -633,7 +637,7 @@
 void
 UnityMTGrabHandlesScreen::addHandleWindow(const unity::MT::GrabHandle::Ptr &h, Window w)
 {
-  mInputHandles.insert(std::pair <Window, const unity::MT::GrabHandle::Ptr> (w, h));
+  mInputHandles.insert(std::make_pair(w, h));
 }
 
 void
@@ -774,7 +778,7 @@
     GLTexture::List t = GLTexture::readImageToTexture(fname, pname,
                                                       size);
 
-    (boost::shared_static_cast <unity::MT::X11TextureFactory> (unity::MT::Texture::Factory::Default ()))->setActiveWrap (t);
+    (static_cast<unity::MT::X11TextureFactory*>(unity::MT::Texture::Factory::Default().get())->setActiveWrap(t));
 
     mHandleTextures.at(i).first = unity::MT::Texture::Factory::Default ()->create ();
     mHandleTextures.at (i).second.width = size.width ();

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.h	2012-09-13 10:56:42 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-grab-handles.h	2012-10-13 20:30:29 +0000
@@ -22,9 +22,7 @@
 #include <composite/composite.h>
 #include <opengl/opengl.h>
 #include <boost/noncopyable.hpp>
-#include <boost/shared_ptr.hpp>
-#include <boost/enable_shared_from_this.hpp>
-#include <boost/weak_ptr.hpp>
+#include <memory>
 
 #include <core/atoms.h>
 
@@ -72,7 +70,7 @@
 {
   public:
 
-    typedef boost::shared_ptr <X11Texture> Ptr;
+    typedef std::shared_ptr <X11Texture> Ptr;
 
     X11Texture (const GLTexture::List &t);
 
@@ -124,7 +122,7 @@
 
 private:
 
-  boost::weak_ptr <unity::MT::GrabHandle>  mGrabHandle;
+  std::weak_ptr <unity::MT::GrabHandle>  mGrabHandle;
   Window                                   mIpw;
   Display                                  *mDpy;
 };
@@ -175,7 +173,7 @@
   void preparePaint(int);
   void donePaint();
 
-  void raiseHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &,
+  void raiseHandle (const std::shared_ptr <const unity::MT::GrabHandle> &,
                     Window                      owner);
 
   std::vector <unity::MT::TextureSize>  & textures()
@@ -188,7 +186,7 @@
   std::list <unity::MT::GrabHandleGroup::Ptr> mGrabHandles;
   std::vector <unity::MT::TextureSize> mHandleTextures;
 
-  std::map <Window, const unity::MT::GrabHandle::Ptr> mInputHandles;
+  std::map <Window, const std::weak_ptr <unity::MT::GrabHandle> > mInputHandles;
   CompWindowVector         		    mLastClientListStacking;
   Atom             			    mCompResizeWindowAtom;
 
@@ -243,7 +241,7 @@
 
 protected:
 
-  void raiseGrabHandle (const boost::shared_ptr <const unity::MT::GrabHandle> &h);
+  void raiseGrabHandle (const std::shared_ptr <const unity::MT::GrabHandle> &h);
   void requestMovement (int x,
                         int y,
                         unsigned int direction,

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-texture.cpp'
--- plugins/unity-mt-grab-handles/src/unity-mt-texture.cpp	2011-12-23 07:19:07 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-texture.cpp	2012-10-13 20:30:29 +0000
@@ -18,7 +18,7 @@
 
 #include "unity-mt-texture.h"
 
-boost::shared_ptr <unity::MT::Texture::Factory> unity::MT::Texture::Factory::mDefault;
+std::shared_ptr <unity::MT::Texture::Factory> unity::MT::Texture::Factory::mDefault;
 
 unity::MT::Texture::Factory::Factory ()
 {
@@ -34,7 +34,7 @@
   mDefault.reset (f);
 }
 
-boost::shared_ptr <unity::MT::Texture::Factory>
+std::shared_ptr <unity::MT::Texture::Factory>
 unity::MT::Texture::Factory::Default ()
 {
   return mDefault;

=== modified file 'plugins/unity-mt-grab-handles/src/unity-mt-texture.h'
--- plugins/unity-mt-grab-handles/src/unity-mt-texture.h	2011-12-23 07:19:07 +0000
+++ plugins/unity-mt-grab-handles/src/unity-mt-texture.h	2012-10-13 20:30:29 +0000
@@ -21,7 +21,6 @@
 
 #include <Nux/Nux.h>
 #include <boost/noncopyable.hpp>
-#include <boost/shared_ptr.hpp>
 
 namespace unity
 {
@@ -31,7 +30,7 @@
 {
   public:
 
-    typedef boost::shared_ptr <Texture> Ptr;
+    typedef std::shared_ptr <Texture> Ptr;
 
     virtual ~Texture ();
 
@@ -48,7 +47,7 @@
       static void
       SetDefault (Factory *);
 
-      static boost::shared_ptr <Factory>
+      static std::shared_ptr <Factory>
       Default ();
 
     protected:
@@ -57,7 +56,7 @@
 
     private:
 
-      static boost::shared_ptr <unity::MT::Texture::Factory> mDefault;
+      static std::shared_ptr <unity::MT::Texture::Factory> mDefault;
     };
 
   protected:

=== modified file 'tests/test_grabhandle.cpp'
--- tests/test_grabhandle.cpp	2011-12-23 01:28:05 +0000
+++ tests/test_grabhandle.cpp	2012-10-13 20:30:29 +0000
@@ -7,7 +7,7 @@
 #include <unity-mt-grab-handle-group.h>
 #include <unity-mt-grab-handle-impl-factory.h>
 #include <unity-mt-texture.h>
-#include <boost/shared_ptr.hpp>
+#include <memory>
 
 unsigned int unity::MT::MaximizedHorzMask = (1 << 0);
 unsigned int unity::MT::MaximizedVertMask = (1 << 1);
@@ -95,7 +95,7 @@
 class MockGrabHandleTexture : public unity::MT::Texture
 {
 public:
-  typedef boost::shared_ptr <MockGrabHandleTexture> Ptr;
+  typedef std::shared_ptr <MockGrabHandleTexture> Ptr;
   MockGrabHandleTexture () : unity::MT::Texture () {};
 };
 
@@ -112,13 +112,14 @@
 public:
   MockGrabHandleWindow () : GrabHandleWindow () {};
   MOCK_METHOD4 (requestMovement, void (int, int, unsigned int, unsigned int));
-  MOCK_METHOD1 (raiseGrabHandle, void (const boost::shared_ptr <const unity::MT::GrabHandle> &));
+  MOCK_METHOD1 (raiseGrabHandle, void (const std::shared_ptr <const unity::MT::GrabHandle> &));
 };
 
 Texture::Ptr
 MockGrabHandleTextureFactory::create ()
 {
-  return boost::shared_static_cast <Texture> (MockGrabHandleTexture::Ptr (new MockGrabHandleTexture ()));
+  Texture::Ptr pt(static_cast<unity::MT::Texture*>(new MockGrabHandleTexture()));
+  return pt;
 }
 
 GrabHandle::Impl *