Merge lp:~alan-griffiths/compiz-core/Bug-931283 into lp:compiz-core

[Alan Griffiths]

Fix teardown sequence

[Alan Griffiths (alan-griffiths)]

This *might* also fix the hang on "--replace" that Sam mentioned on chat. (Not sure how to reproduce that.)

[Daniel van Vugt (vanvugt)]

Using this fix, now it crashes in opengl shutdown instead. But the stacks are similar to the original bug...

==14834== Invalid read of size 8
==14834== at 0x9E59307: OpenglPluginVTable::fini() (opengl.cpp:99)
==14834== by 0x4E9F3BD: CompPlugin::pop() (plugin.cpp:524)
==14834== by 0x4E6B913: PrivateScreen::~PrivateScreen() (screen.cpp:5021)
==14834== by 0x4E6C0B8: PrivateScreen::~PrivateScreen() (screen.cpp:5040)
==14834== by 0x4E68041: CompScreenImpl::~CompScreenImpl() (checked_delete.hpp:34)
==14834== by 0x4E681A8: CompScreenImpl::~CompScreenImpl() (screen.cpp:4930)
==14834== by 0x402CC2: CompManager::fini() (main.cpp:214)
==14834== by 0x4029A5: main (main.cpp:239)
==14834== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==14834==
==14834==
==14834== Process terminating with default action of signal 11 (SIGSEGV)
==14834== Access not within mapped region at address 0x0
==14834== at 0x9E59307: OpenglPluginVTable::fini() (opengl.cpp:99)
==14834== by 0x4E9F3BD: CompPlugin::pop() (plugin.cpp:524)
==14834== by 0x4E6B913: PrivateScreen::~PrivateScreen() (screen.cpp:5021)
==14834== by 0x4E6C0B8: PrivateScreen::~PrivateScreen() (screen.cpp:5040)
==14834== by 0x4E68041: CompScreenImpl::~CompScreenImpl() (checked_delete.hpp:34)
==14834== by 0x4E681A8: CompScreenImpl::~CompScreenImpl() (screen.cpp:4930)
==14834== by 0x402CC2: CompManager::fini() (main.cpp:214)
==14834== by 0x4029A5: main (main.cpp:239)
==14834== If you believe this happened as a result of a stack
==14834== overflow in your program's main thread (unlikely but
==14834== possible), you can try to increase the size of the
==14834== main thread stack using the --main-stacksize= flag.
==14834== The main thread stack size used in this run was 8388608.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

Didn't see that - will look harder.

[lp:~alan-griffiths/compiz-core/Bug-931283]

2999. By Alan Griffiths on 2012-02-13

Fix teardown sequence

[Alan Griffiths (alan-griffiths)]

Reproduced opengl problem and fixed.

[lp:~alan-griffiths/compiz-core/Bug-931283]

3000. By Alan Griffiths on 2012-02-13

Fix invalid memory read reported by valgrind

[Sam Spilsbury (smspillaz)]

The code looks fine - if this is shutting down cleanly, please merge it alan.

review: Approve

[Daniel van Vugt (vanvugt)]

Confirmed. All the crashes are fixed in revision 3000. Valgrind is happy.

review: Approve

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-02-13 08:15:12 +0000
+++ src/screen.cpp	2012-02-13 11:56:23 +0000
@@ -4660,8 +4660,8 @@
 					      compScreenSnEvent, this, NULL);
 
     wmSnSelectionWindow = newWmSnOwner;
-    wmSnAtom            = wmSnAtom;
-    wmSnTimestamp       = wmSnTimestamp;
+    this->wmSnAtom            = wmSnAtom;
+    this->wmSnTimestamp       = wmSnTimestamp;
 
     if (!XGetWindowAttributes (dpy, root, &attrib))
 	return false;
@@ -4921,6 +4921,14 @@
 
 CompScreenImpl::~CompScreenImpl ()
 {
+    priv->removeAllSequences ();
+
+    while (!priv->windows.empty ())
+        delete priv->windows.front ();
+
+    while (CompPlugin* p = CompPlugin::pop ())
+	CompPlugin::unload (p);
+
     screen = NULL;
 }
 
@@ -4996,11 +5004,6 @@
 {
     if (initialized)
     {
-	removeAllSequences ();
-
-	while (!windows.empty ())
-	    delete windows.front ();
-
 	XUngrabKey (dpy, AnyKey, AnyModifier, root);
 
 	initialized = false;
@@ -5015,18 +5018,16 @@
 
 	XFreeCursor (dpy, invisibleCursor);
 	XSync (dpy, False);
+
+	if (snContext)
+	    sn_monitor_context_unref (snContext);
+
 	XCloseDisplay (dpy);
     }
 
-    while (CompPlugin* p = CompPlugin::pop ())
-	CompPlugin::unload (p);
-
     if (desktopHintData)
 	free (desktopHintData);
 
-    if (snContext)
-	sn_monitor_context_unref (snContext);
-
     if (snDisplay)
 	sn_display_unref (snDisplay);