Merge lp:~thomir/unity/panelmenuview-fix-invalid-read into lp:unity

[Thomi Richards]

PanelMenuView no longer tries to dereference a pointer that has already been deleted when it's destroyed.

Problem:

PanelMenuView has an Animator instance as a member variable. It conntacts the animator's 'animation_finished' signal to PanelMenuView::FullRedraw. When the PanelMenuView dies, the animator calls 'Stop' in it's destructor, which triggers the animation_ended signal (if the animation was running). This in turn calls PanelMenuView::FullRedraw, which tries to access the _window_buttons pointer which has already been deleted. Phew!

Solution:

Explicitly stop the animation in the PanelMenuView destructor BEFORE the _window_buttons member variable is deleted. The Animator class is clever enough not to emit the 'animation_finished' signal if the animation has already been stopped. In order to guard against this happening again in the future, the destructor now sets it's pointer member variables to nullptr after they've been destroyed.

UNBLOCK

[Brandon Schaefer (brandontschaefer)]

Looks good! +1

review: Approve

[Tim Penhey (thumper)]

Please don't assign to nullptr in the destructor. It is pointless.

review: Needs Fixing

[lp:~thomir/unity/panelmenuview-fix-invalid-read]

2264. By Thomi Richards on 2012-04-11

Found a better way.

2265. By Thomi Richards on 2012-04-11

Removed nullptr assigns.

[Tim Penhey (thumper)]

review: Approve

=== modified file 'plugins/unityshell/src/PanelMenuView.cpp'
--- plugins/unityshell/src/PanelMenuView.cpp	2012-04-09 12:35:20 +0000
+++ plugins/unityshell/src/PanelMenuView.cpp	2012-04-11 00:40:24 +0000
@@ -173,6 +173,15 @@
 
 PanelMenuView::~PanelMenuView()
 {
+  // We need to disconnect these signals explicitly before we destroy the window buttons
+  // and titlebar grab area objects, otherwise there's a risk the signals will fire as the
+  // animator objects are destroyed (which happens after the destructor finishes).
+  _fade_in_animator.animation_updated.clear();
+  _fade_in_animator.animation_ended.clear();
+  _fade_out_animator.animation_updated.clear();
+  _fade_out_animator.animation_ended.clear();
+  _style_changed_connection.disconnect();
+
   if (_active_moved_id)
     g_source_remove(_active_moved_id);
 
@@ -184,8 +193,6 @@
 
   _window_buttons->UnReference();
   _titlebar_grab_area->UnReference();
-
-  _style_changed_connection.disconnect();
 }
 
 void PanelMenuView::OverlayShown()