Merge lp:~compiz-team/compiz/compiz.fix_927168 into lp:compiz/0.9.8

[Sam Spilsbury]

Check that pixmaps which aren't managed by us actually exist before binding.

It was possible for there to be a race condition for a pixmap to become
invalid on the server side if the client which did have control over thier
lifecycle freed them before we were done with them or disconnected. Drivers
have normally handled this condition by reading the contents of the undefined
memory directly without an error, however drivers such as LLVMpipe will
effectively dereference an invalid pointer and crash when you do this.

LP: #927168:
compiz crashed with SIGSEGV in
memmove() from
drisw_update_tex_buffer() from
dri_set_tex_buffer2() from
drisw_bind_tex_image() from
__glXBindTexImageEXT() from
TfpTexture::enable()

Most of the time we don't need to employ this check. Its only when we know
that we don't control the lifecycle of a pixmap that race conditions
such as these can occur.

== Change:

    Check that pixmaps which aren't managed by us actually exist before binding.

    It was possible for there to be a race condition for a pixmap to become
    invalid on the server side if the client which did have control over thier
    lifecycle freed them before we were done with them or disconnected. Drivers
    have normally handled this condition by reading the contents of the undefined
    memory directly without an error, however drivers such as LLVMpipe will
    effectively dereference an invalid pointer and crash when you do this.

    Eg:

    compiz crashed with SIGSEGV in memmove() from drisw_update_tex_buffer() from dri_set_tex_buffer2() from drisw_bind_tex_image() from __glXBindTexImageEXT() from TfpTexture::enable()

    Most of the time we don't need to employ this check. Its only when we know
    that we don't control the lifecycle of a pixmap that race conditions
    such as these can occurr.

== Test

    Five unit tests included. Testing that this fix can be effective can be done as follows:

    1. Start compiz with llvmpipe (eg LIBGL_ALWAYS_SOFTWARE=1)
    2. Set a breakpoint on TfpTexture::bindTexImage
    3. Hover over the decoration buttons so as to cause a damage event (and a require rebind)
    4. Once the breakpoint is hit, run killall -9 gtk-window-decorator
    5. Continue the program. It shouldn't crash.

[lp:~compiz-team/compiz/compiz.fix_927168]

3380. By Sam Spilsbury on 2012-09-17

Actually provide the hint

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:3379
http://jenkins.qa.ubuntu.com/job/compiz-ci/80/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/80/console

review: Approve (continuous-integration)

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:3380
http://jenkins.qa.ubuntu.com/job/compiz-ci/81/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/81/console

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I can't find any obvious regressions and valgrind is happy.

This bug is a very big deal and causing constant duplicates, so I won't let any style complaints get in the way of approving it.

review: Approve

=== modified file 'plugins/copytex/src/copytex.cpp'
--- plugins/copytex/src/copytex.cpp	2012-09-07 22:37:20 +0000
+++ plugins/copytex/src/copytex.cpp	2012-09-17 15:27:20 +0000
@@ -37,10 +37,11 @@
 };
 
 GLTexture::List
-CopyPixmap::bindPixmapToTexture (Pixmap pixmap,
-				 int width,
-				 int height,
-				 int depth)
+CopyPixmap::bindPixmapToTexture (Pixmap                       pixmap,
+				 int                          width,
+				 int                          height,
+				 int                          depth,
+				 compiz::opengl::PixmapSource source)
 {
     if (depth != 32 && depth != 24)
 	return GLTexture::List ();

=== modified file 'plugins/copytex/src/copytex.h'
--- plugins/copytex/src/copytex.h	2012-09-07 22:37:20 +0000
+++ plugins/copytex/src/copytex.h	2012-09-17 15:27:20 +0000
@@ -52,10 +52,11 @@
 
 	~CopyPixmap ();
 
-	static GLTexture::List bindPixmapToTexture (Pixmap pixmap,
-						    int width,
-						    int height,
-						    int depth);
+	static GLTexture::List bindPixmapToTexture (Pixmap                       pixmap,
+						    int                          width,
+						    int                          height,
+						    int                          depth,
+						    compiz::opengl::PixmapSource source);
 
     public:
 	Textures textures;

=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2012-09-13 10:58:13 +0000
+++ plugins/decor/src/decor.cpp	2012-09-17 15:27:20 +0000
@@ -337,7 +337,9 @@
     }
 
     bindFailed = false;
-    textures = GLTexture::bindPixmapToTexture (pixmap->getPixmap (), width, height, depth);
+    textures = GLTexture::bindPixmapToTexture (pixmap->getPixmap (),
+					       width, height, depth,
+					       compiz::opengl::ExternallyManaged);
     if (textures.size () != 1)
     {
 	bindFailed = true;

=== modified file 'plugins/opengl/CMakeLists.txt'
--- plugins/opengl/CMakeLists.txt	2012-09-04 10:19:19 +0000
+++ plugins/opengl/CMakeLists.txt	2012-09-17 15:27:20 +0000
@@ -5,10 +5,14 @@
 set (INTERNAL_LIBRARIES
     compiz_opengl_double_buffer
     compiz_opengl_fsregion
+    compiz_opengl_glx_tfp_bind
 )
 
 add_subdirectory (src/doublebuffer)
 add_subdirectory (src/fsregion)
+add_subdirectory (src/glxtfpbind)
+
+include_directories (src/glxtfpbind/include)
 
 if (USE_GLES)
     compiz_plugin(opengl PLUGINDEPS composite CFLAGSADD "-DUSE_GLES -std=c++0x" LIBRARIES ${OPENGLES2_LIBRARIES} ${INTERNAL_LIBRARIES} dl INCDIRS ${OPENGLES2_INCLUDE_DIR})

=== modified file 'plugins/opengl/include/opengl/opengl.h'
--- plugins/opengl/include/opengl/opengl.h	2012-09-07 23:29:42 +0000
+++ plugins/opengl/include/opengl/opengl.h	2012-09-17 15:27:20 +0000
@@ -50,7 +50,7 @@
 #include <opengl/programcache.h>
 #include <opengl/shadercache.h>
 
-#define COMPIZ_OPENGL_ABI 5
+#define COMPIZ_OPENGL_ABI 6
 
 /*
  * Some plugins check for #ifdef USE_MODERN_COMPIZ_GL. Support it for now, but

=== added file 'plugins/opengl/include/opengl/pixmapsource.h'
--- plugins/opengl/include/opengl/pixmapsource.h	1970-01-01 00:00:00 +0000
+++ plugins/opengl/include/opengl/pixmapsource.h	2012-09-17 15:27:20 +0000
@@ -0,0 +1,40 @@
+/*
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Authors: Sam Spilsbury <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef _COMPIZ_OPENGL_PIXMAP_SOURCE_H
+#define _COMPIZ_OPENGL_PIXMAP_SOURCE_H
+
+namespace compiz
+{
+    namespace opengl
+    {
+	typedef enum _PixmapSource
+	{
+	    InternallyManaged = 0,
+	    ExternallyManaged = 1
+	} PixmapSource;
+    }
+}
+
+#endif

=== modified file 'plugins/opengl/include/opengl/texture.h'
--- plugins/opengl/include/opengl/texture.h	2012-05-17 10:41:21 +0000
+++ plugins/opengl/include/opengl/texture.h	2012-09-17 15:27:20 +0000
@@ -43,6 +43,8 @@
 
 #include <vector>
 
+#include <opengl/pixmapsource.h>
+
 
 #define POWER_OF_TWO(v) ((v & (v - 1)) == 0)
 
@@ -104,7 +106,7 @@
 		void clear ();
 	};
 
-	typedef boost::function<List (Pixmap, int, int, int)> BindPixmapProc;
+	typedef boost::function<List (Pixmap, int, int, int, compiz::opengl::PixmapSource)> BindPixmapProc;
 	typedef unsigned int BindPixmapHandle;
 
     public:
@@ -181,11 +183,14 @@
 	 * @param width Specifies the width of the texture
 	 * @param height Specifies the height of the texture
 	 * @param depth Specifies the color depth of the texture
+	 * @param source Whether the pixmap lifecycle is managed externall
 	 */
-	static List bindPixmapToTexture (Pixmap pixmap,
-					 int width,
-					 int height,
-					 int depth);
+	static List bindPixmapToTexture (Pixmap                       pixmap,
+					 int                          width,
+					 int                          height,
+					 int                          depth,
+					 compiz::opengl::PixmapSource source
+					     = compiz::opengl::InternallyManaged);
 
 	/**
 	 * Returns a GLTexture::List with the contents of of

=== added directory 'plugins/opengl/src/glxtfpbind'
=== added file 'plugins/opengl/src/glxtfpbind/CMakeLists.txt'
--- plugins/opengl/src/glxtfpbind/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/CMakeLists.txt	2012-09-17 15:27:20 +0000
@@ -0,0 +1,31 @@
+INCLUDE_DIRECTORIES (  
+  ${compiz_SOURCE_DIR}/src/servergrab/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/../../include
+  ${CMAKE_CURRENT_SOURCE_DIR}/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/src
+    
+  ${Boost_INCLUDE_DIRS}
+)
+
+LINK_DIRECTORIES (${COMPIZ_LIBRARY_DIRS}) 
+
+SET( 
+  SRCS 
+  ${CMAKE_CURRENT_SOURCE_DIR}/src/glx-tfp-bind.cpp
+)
+
+ADD_LIBRARY( 
+  compiz_opengl_glx_tfp_bind STATIC
+  
+  ${SRCS}
+)
+
+if (COMPIZ_BUILD_TESTING)
+ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
+endif (COMPIZ_BUILD_TESTING)
+
+TARGET_LINK_LIBRARIES(
+  compiz_opengl_glx_tfp_bind
+
+  compiz_servergrab
+)

=== added directory 'plugins/opengl/src/glxtfpbind/include'
=== added file 'plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h'
--- plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h	2012-09-17 15:27:20 +0000
@@ -0,0 +1,53 @@
+/*
+ * Compiz, opengl plugin, GLX_EXT_texture_from_pixmap rebind logic
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Authors: Sam Spilsbury <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#ifndef _COMPIZ_OPENGL_GLX_TFP_BIND_H
+#define _COMPIZ_OPENGL_GLX_TFP_BIND_H
+
+#include <opengl/pixmapsource.h>
+#include <boost/function.hpp>
+
+class ServerGrabInterface;
+typedef unsigned long Pixmap;
+typedef unsigned long GLXPixmap;
+
+namespace compiz
+{
+    namespace opengl
+    {
+	typedef boost::function <bool (Pixmap)> PixmapCheckValidityFunc;
+	typedef boost::function <void (GLXPixmap)> BindTexImageEXTFunc;
+	typedef boost::function <void ()> WaitGLXFunc;
+
+	bool bindTexImageGLX (ServerGrabInterface           *,
+			      Pixmap,
+			      GLXPixmap,
+			      const PixmapCheckValidityFunc &,
+			      const BindTexImageEXTFunc     &,
+			      const WaitGLXFunc             &,
+			      PixmapSource);
+			      
+    } // namespace opengl
+} // namespace compiz
+#endif

=== added directory 'plugins/opengl/src/glxtfpbind/src'
=== added file 'plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp'
--- plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp	2012-09-17 15:27:20 +0000
@@ -0,0 +1,53 @@
+/*
+ * Compiz, opengl plugin, GLX_EXT_texture_from_pixmap rebind logic
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Authors: Sam Spilsbury <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#include <opengl/pixmapsource.h>
+#include <core/servergrab.h>
+#include "glx-tfp-bind.h"
+
+namespace cgl = compiz::opengl;
+
+bool
+cgl::bindTexImageGLX (ServerGrabInterface                *serverGrabInterface,
+		      Pixmap                             x11Pixmap,
+		      GLXPixmap                          glxPixmap,
+		      const cgl::PixmapCheckValidityFunc &checkPixmapValidity,
+		      const cgl::BindTexImageEXTFunc     &bindTexImageEXT,
+		      const cgl::WaitGLXFunc             &waitGLX,
+		      cgl::PixmapSource                  source)
+{
+    ServerLock lock (serverGrabInterface);
+
+    waitGLX ();
+
+    /* External pixmaps can disappear on us, but not
+     * while we have a server grab at least */
+    if (source == cgl::ExternallyManaged)
+	if (!checkPixmapValidity (x11Pixmap))
+	    return false;
+
+    bindTexImageEXT (glxPixmap);
+
+    return true;
+}

=== added directory 'plugins/opengl/src/glxtfpbind/tests'
=== added file 'plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt'
--- plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt	2012-09-17 15:27:20 +0000
@@ -0,0 +1,24 @@
+find_library (GMOCK_LIBRARY gmock)
+find_library (GMOCK_MAIN_LIBRARY gmock_main)
+
+if (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+  message ("Google Mock and Google Test not found - cannot build tests!")
+  set (COMPIZ_BUILD_TESTING OFF)
+endif (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+
+include_directories (${GTEST_INCLUDE_DIRS})
+
+link_directories (${COMPIZ_LIBRARY_DIRS})
+
+add_executable (compiz_test_opengl_glx_tfp_bind
+                ${CMAKE_CURRENT_SOURCE_DIR}/test-opengl-glx-tfp-bind.cpp)
+
+target_link_libraries (compiz_test_opengl_glx_tfp_bind
+                       compiz_opengl_glx_tfp_bind
+                       ${GTEST_BOTH_LIBRARIES}
+		       ${GMOCK_LIBRARY}
+		       ${GMOCK_MAIN_LIBRARY}
+		       ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
+                       )
+
+compiz_discover_tests (compiz_test_opengl_glx_tfp_bind COVERAGE compiz_opengl_glx_tfp_bind)

=== added file 'plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp'
--- plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp	2012-09-17 15:27:20 +0000
@@ -0,0 +1,157 @@
+#include <boost/bind.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <core/servergrab.h>
+#include "glx-tfp-bind.h"
+
+using ::testing::InSequence;
+using ::testing::NiceMock;
+using ::testing::StrictMock;
+using ::testing::Return;
+
+namespace cgl = compiz::opengl;
+
+namespace
+{
+    const Pixmap pixmap = 1;
+    const GLXPixmap glxPixmap = 2;
+
+    void emptyWaitGLX () {}
+    bool emptyCheckPixmap (Pixmap p) { return true; }
+    void emptyBindTexImage (GLXPixmap p) {}
+
+    cgl::WaitGLXFunc waitGLX () { return boost::bind (emptyWaitGLX); }
+    cgl::PixmapCheckValidityFunc pixmapCheckValidity () { return boost::bind (emptyCheckPixmap, _1); }
+    cgl::BindTexImageEXTFunc bindTexImageEXT () { return boost::bind (emptyBindTexImage, _1); }
+}
+
+class MockWaitGLX
+{
+    public:
+
+	MOCK_METHOD0 (waitGLX, void ());
+};
+
+class MockPixmapCheckValidity
+{
+    public:
+
+	MOCK_METHOD1 (checkValidity, bool (Pixmap));
+};
+
+class MockBindTexImageEXT
+{
+    public:
+
+	MOCK_METHOD1 (bindTexImageEXT, void (GLXPixmap));
+};
+
+class MockServerGrab :
+    public ServerGrabInterface
+{
+    public:
+
+	MOCK_METHOD0 (grabServer, void ());
+	MOCK_METHOD0 (syncServer, void ());
+	MOCK_METHOD0 (ungrabServer, void ());
+};
+
+TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestTakesServerGrab)
+{
+    MockServerGrab mockServerGrab;
+    InSequence     s;
+
+    EXPECT_CALL (mockServerGrab, grabServer ());
+    EXPECT_CALL (mockServerGrab, syncServer ());
+    EXPECT_CALL (mockServerGrab, ungrabServer ());
+    EXPECT_CALL (mockServerGrab, syncServer ());
+
+    cgl::bindTexImageGLX (&mockServerGrab,
+			  pixmap,
+			  glxPixmap,
+			  pixmapCheckValidity (),
+			  bindTexImageEXT (),
+			  waitGLX (),
+			  cgl::InternallyManaged);
+}
+
+TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCallsWaitGLX)
+{
+    NiceMock <MockServerGrab> mockServerGrab;
+    MockWaitGLX               mockWaitGLX;
+
+    cgl::WaitGLXFunc          waitGLXFuncMock (boost::bind (&MockWaitGLX::waitGLX, &mockWaitGLX));
+
+    EXPECT_CALL (mockWaitGLX, waitGLX ());
+
+    cgl::bindTexImageGLX (&mockServerGrab,
+			  pixmap,
+			  glxPixmap,
+			  pixmapCheckValidity (),
+			  bindTexImageEXT (),
+			  waitGLXFuncMock,
+			  cgl::InternallyManaged);
+}
+
+TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestNoCallToCheckValidityIfInternalAndImmediateBind)
+{
+    NiceMock <MockServerGrab>            mockServerGrab;
+    StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
+    StrictMock <MockBindTexImageEXT>     mockBindTexImage;
+
+    cgl::PixmapCheckValidityFunc         pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
+    cgl::BindTexImageEXTFunc             bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
+
+    EXPECT_CALL (mockBindTexImage, bindTexImageEXT (glxPixmap));
+
+    EXPECT_TRUE (cgl::bindTexImageGLX (&mockServerGrab,
+				       pixmap,
+				       glxPixmap,
+				       pixmapCheckFunc,
+				       bindTexImageEXTFunc,
+				       waitGLX (),
+				       cgl::InternallyManaged));
+}
+
+TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCheckValidityIfExternalNoBindIfInvalid)
+{
+    NiceMock <MockServerGrab>            mockServerGrab;
+    StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
+    StrictMock <MockBindTexImageEXT>     mockBindTexImage;
+
+    cgl::PixmapCheckValidityFunc         pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
+    cgl::BindTexImageEXTFunc             bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
+
+    EXPECT_CALL (mockPixmapCheck, checkValidity (pixmap)).WillOnce (Return (false));
+
+    EXPECT_FALSE (cgl::bindTexImageGLX (&mockServerGrab,
+					pixmap,
+					glxPixmap,
+					pixmapCheckFunc,
+					bindTexImageEXTFunc,
+					waitGLX (),
+					cgl::ExternallyManaged));
+}
+
+TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCheckValidityIfExternalBindIfValid)
+{
+    NiceMock <MockServerGrab>            mockServerGrab;
+    StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
+    StrictMock <MockBindTexImageEXT>     mockBindTexImage;
+
+    cgl::PixmapCheckValidityFunc         pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
+    cgl::BindTexImageEXTFunc             bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
+
+    EXPECT_CALL (mockPixmapCheck, checkValidity (pixmap)).WillOnce (Return (true));
+    EXPECT_CALL (mockBindTexImage, bindTexImageEXT (glxPixmap));
+
+    EXPECT_TRUE (cgl::bindTexImageGLX (&mockServerGrab,
+				       pixmap,
+				       glxPixmap,
+				       pixmapCheckFunc,
+				       bindTexImageEXTFunc,
+				       waitGLX (),
+				       cgl::ExternallyManaged));
+}

=== modified file 'plugins/opengl/src/privatetexture.h'
--- plugins/opengl/src/privatetexture.h	2012-07-04 07:07:37 +0000
+++ plugins/opengl/src/privatetexture.h	2012-09-17 15:27:20 +0000
@@ -79,10 +79,11 @@
 
 	void enable (Filter filter);
 
-	static List bindPixmapToTexture (Pixmap pixmap,
-					 int width,
-					 int height,
-					 int depth);
+	static List bindPixmapToTexture (Pixmap                       pixmap,
+					 int                          width,
+					 int                          height,
+					 int                          depth,
+					 compiz::opengl::PixmapSource source);
 
     public:
 	bool        damaged;
@@ -102,16 +103,19 @@
 	bool bindTexImage (const GLXPixmap &);
 	void releaseTexImage ();
 
-	static List bindPixmapToTexture (Pixmap pixmap,
-					 int width,
-					 int height,
-					 int depth);
+	static List bindPixmapToTexture (Pixmap                       pixmap,
+					 int                          width,
+					 int                          height,
+					 int                          depth,
+					 compiz::opengl::PixmapSource source);
 
     public:
-	GLXPixmap pixmap;
-	bool      damaged;
-	Damage    damage;
-	bool      updateMipMap;
+	Pixmap                       x11Pixmap;
+	GLXPixmap                    pixmap;
+	bool                         damaged;
+	Damage                       damage;
+	bool                         updateMipMap;
+	compiz::opengl::PixmapSource source;
 };
 
 extern std::map<Damage, TfpTexture*> boundPixmapTex;

=== modified file 'plugins/opengl/src/texture.cpp'
--- plugins/opengl/src/texture.cpp	2012-08-13 07:31:34 +0000
+++ plugins/opengl/src/texture.cpp	2012-09-17 15:27:20 +0000
@@ -31,11 +31,17 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <boost/scoped_ptr.hpp>
+
 #include <core/servergrab.h>
 #include <opengl/texture.h>
 #include <privatetexture.h>
 #include "privates.h"
 
+#include "glx-tfp-bind.h"
+
+namespace cgl = compiz::opengl;
+
 #ifdef USE_GLES
 std::map<Damage, EglTexture*> boundPixmapTex;
 #else
@@ -421,23 +427,58 @@
 }
 
 GLTexture::List
-GLTexture::bindPixmapToTexture (Pixmap pixmap,
-				int    width,
-				int    height,
-				int    depth)
+GLTexture::bindPixmapToTexture (Pixmap                       pixmap,
+				int                          width,
+				int                          height,
+				int                          depth,
+				compiz::opengl::PixmapSource source)
 {
     GLTexture::List rv;
 
     foreach (BindPixmapProc &proc, GLScreen::get (screen)->priv->bindPixmap)
     {
 	if (!proc.empty ())
-	    rv = proc (pixmap, width, height, depth);
+	    rv = proc (pixmap, width, height, depth, source);
 	if (rv.size ())
 	    return rv;
     }
     return GLTexture::List ();
 }
 
+namespace
+{
+    bool checkPixmapValidityGLX (Pixmap pixmap)
+    {
+	Window       windowReturn;
+	unsigned int uiReturn;
+	int          iReturn;
+
+	if (!XGetGeometry (screen->dpy (),
+			   pixmap,
+			   &windowReturn,
+			   &iReturn,
+			   &iReturn,
+			   &uiReturn,
+			   &uiReturn,
+			   &uiReturn,
+			   &uiReturn))
+	    return false;
+	return true;
+    }
+
+    const cgl::WaitGLXFunc & waitGLXFunc ()
+    {
+	static const cgl::WaitGLXFunc f (boost::bind (glXWaitX));
+	return f;
+    }
+
+    const cgl::PixmapCheckValidityFunc & checkPixmapValidityFunc ()
+    {
+	static const cgl::PixmapCheckValidityFunc f (boost::bind (checkPixmapValidityGLX, _1));
+	return f;
+    }
+}
+
 #ifdef USE_GLES
 EglTexture::EglTexture () :
     damaged (true),
@@ -538,6 +579,20 @@
 }
 #else
 
+namespace
+{
+    const cgl::BindTexImageEXTFunc & bindTexImageEXT ()
+    {
+	static int                            *attrib_list = NULL;
+	static const cgl::BindTexImageEXTFunc f (boost::bind (GL::bindTexImage,
+							      screen->dpy (),
+							      _1,
+							      GLX_FRONT_LEFT_EXT,
+							      attrib_list));
+	return f;
+    }
+}
+
 TfpTexture::TfpTexture () :
     pixmap (0),
     damaged (true),
@@ -569,10 +624,13 @@
 bool
 TfpTexture::bindTexImage (const GLXPixmap &glxPixmap)
 {
-    ServerLock sg (screen->serverGrabInterface ());
-    glXWaitX ();
-    (*GL::bindTexImage) (screen->dpy (), glxPixmap, GLX_FRONT_LEFT_EXT, NULL);
-    return true;
+    return compiz::opengl::bindTexImageGLX (screen->serverGrabInterface (),
+					    x11Pixmap,
+					    glxPixmap,
+					    checkPixmapValidityFunc (),
+					    bindTexImageEXT (),
+					    waitGLXFunc (),
+					    source);
 }
 
 void
@@ -582,10 +640,11 @@
 }
 
 GLTexture::List
-TfpTexture::bindPixmapToTexture (Pixmap pixmap,
-				 int    width,
-				 int    height,
-				 int    depth)
+TfpTexture::bindPixmapToTexture (Pixmap            pixmap,
+				 int               width,
+				 int               height,
+				 int               depth,
+				 cgl::PixmapSource source)
 {
     if ((int) width > GL::maxTextureSize || (int) height > GL::maxTextureSize ||
         !GL::textureFromPixmap)
@@ -650,6 +709,19 @@
 
     attribs[i++] = None;
 
+    /* We need to take a server grab here if the pixmap is
+     * externally managed as we need to be able to query
+     * if it actually exists */
+    boost::scoped_ptr <ServerLock> lock;
+
+    if (source == cgl::ExternallyManaged)
+    {
+	lock.reset (new ServerLock (screen->serverGrabInterface ()));
+
+	if (!checkPixmapValidityGLX (pixmap))
+	    return false;
+    }
+
     glxPixmap = (*GL::createPixmap) (screen->dpy (), config->fbConfig,
 				     pixmap, attribs);
 
@@ -711,6 +783,8 @@
     tex->setData (texTarget, matrix, mipmap);
     tex->setGeometry (0, 0, width, height);
     tex->pixmap = glxPixmap;
+    tex->x11Pixmap = pixmap;
+    tex->source = source;
 
     rv[0] = tex;