Merge lp:~alan-griffiths/mir/fix-server-crash into lp:mir

[Alan Griffiths]

Fix the server crash observed in lp:1629275

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3731
https://mir-jenkins.ubuntu.com/job/mir-ci/1870/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2372
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2435
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2427
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2427
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2427
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2401/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2401/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2401/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2401/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2401/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2401
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2401/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1870/rebuild

review: Approve (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3732
https://mir-jenkins.ubuntu.com/job/mir-ci/1871/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2373/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2436
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2428
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2428
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2428
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2402/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2402
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2402/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2402
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2402/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2402
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2402/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2402
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2402/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2402
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2402/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1871/rebuild

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

A test case might be good.

review: Approve

[Andreas Pokorny (andreas-pokorny)]

ok

review: Approve

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3732
https://mir-jenkins.ubuntu.com/job/mir-ci/1880/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2384
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2447
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2439
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2439
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2439
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2413/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2413/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2413/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2413/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2413/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2413
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2413/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1880/rebuild

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

This looks sensible.

Do we have any idea about the underlying failure, though? There should probably be a bug about it.

review: Approve

[Alan Griffiths (alan-griffiths)]

On 05/10/16 02:34, Chris Halse Rogers wrote:
> Do we have any idea about the underlying failure, though? There should probably be a bug about it.

My current (unproven) thinking is that the underlying failure is a race
between the server becoming aware of the client crashing and the driver
becoming aware of the client crashing.

--
Alan Griffiths [phone]
Octopull Ltd http://www.octopull.co.uk/

=== modified file 'src/renderers/gl/renderer.cpp'
--- src/renderers/gl/renderer.cpp	2016-09-19 04:16:15 +0000
+++ src/renderers/gl/renderer.cpp	2016-10-03 16:14:19 +0000
@@ -1,4 +1,4 @@
-/* Copyright © 2013 Canonical Ltd.
+/* Copyright © 2013, 2016 Canonical Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 3,
@@ -28,6 +28,7 @@
 #include "mir/gl/texture_cache.h"
 #include "mir/gl/texture.h"
 #include "mir/log.h"
+#include "mir/report_exception.h"
 
 #define GLM_FORCE_RADIANS
 #include <glm/gtc/matrix_transform.hpp>
@@ -287,70 +288,78 @@
     primitives.clear();
     tessellate(primitives, renderable);
 
-    auto surface_tex = texture_cache->load(renderable);
-
-    typedef struct  // Represents parameters of glBlendFuncSeparate()
-    {
-        GLenum src_rgb, dst_rgb, src_alpha, dst_alpha;
-    } BlendSeparate;
-
-    BlendSeparate client_blend;
-
-    // These renderable method names could be better (see LP: #1236224)
-    if (renderable.shaped())  // Client is RGBA:
-    {
-        client_blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
-                        GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
-    }
-    else if (renderable.alpha() == 1.0f)  // RGBX and no window translucency:
-    {
-        client_blend = {GL_ONE,  GL_ZERO,
-                        GL_ZERO, GL_ONE};  // Avoid using src_alpha!
-    }
-    else
-    {   // Client is RGBX but we also have window translucency.
-        // The texture alpha channel is possibly uninitialized so we must be
-        // careful and avoid using SRC_ALPHA (LP: #1423462).
-        client_blend = {GL_ONE,  GL_ONE_MINUS_CONSTANT_ALPHA,
-                        GL_ZERO, GL_ONE};
-        glBlendColor(0.0f, 0.0f, 0.0f, renderable.alpha());
-    }
-
-    for (auto const& p : primitives)
-    {
-        BlendSeparate blend;
-
-        if (p.tex_id == 0)   // The client surface texture
-        {
-            blend = client_blend;
-            surface_tex->bind();
-        }
-        else   // Some other texture from the shell (e.g. decorations) which
-        {      // is always RGBA (valid SRC_ALPHA).
-            blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
-                     GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
-            glBindTexture(GL_TEXTURE_2D, p.tex_id);
-        }
-
-        glVertexAttribPointer(prog.position_attr, 3, GL_FLOAT,
-                              GL_FALSE, sizeof(mgl::Vertex),
-                              &p.vertices[0].position);
-        glVertexAttribPointer(prog.texcoord_attr, 2, GL_FLOAT,
-                              GL_FALSE, sizeof(mgl::Vertex),
-                              &p.vertices[0].texcoord);
-
-        if (blend.dst_rgb == GL_ZERO)
-        {
-            glDisable(GL_BLEND);
+    // if we fail to load the texture, we need to carry on (part of lp:1629275)
+    try
+    {
+        auto surface_tex = texture_cache->load(renderable);
+
+        typedef struct  // Represents parameters of glBlendFuncSeparate()
+        {
+            GLenum src_rgb, dst_rgb, src_alpha, dst_alpha;
+        } BlendSeparate;
+
+        BlendSeparate client_blend;
+
+        // These renderable method names could be better (see LP: #1236224)
+        if (renderable.shaped())  // Client is RGBA:
+        {
+            client_blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
+                            GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
+        }
+        else if (renderable.alpha() == 1.0f)  // RGBX and no window translucency:
+        {
+            client_blend = {GL_ONE,  GL_ZERO,
+                            GL_ZERO, GL_ONE};  // Avoid using src_alpha!
         }
         else
+        {   // Client is RGBX but we also have window translucency.
+            // The texture alpha channel is possibly uninitialized so we must be
+            // careful and avoid using SRC_ALPHA (LP: #1423462).
+            client_blend = {GL_ONE,  GL_ONE_MINUS_CONSTANT_ALPHA,
+                            GL_ZERO, GL_ONE};
+            glBlendColor(0.0f, 0.0f, 0.0f, renderable.alpha());
+        }
+
+        for (auto const& p : primitives)
         {
-            glEnable(GL_BLEND);
-            glBlendFuncSeparate(blend.src_rgb,   blend.dst_rgb,
-                                blend.src_alpha, blend.dst_alpha);
+            BlendSeparate blend;
+
+            if (p.tex_id == 0)   // The client surface texture
+            {
+                blend = client_blend;
+                surface_tex->bind();
+            }
+            else   // Some other texture from the shell (e.g. decorations) which
+            {      // is always RGBA (valid SRC_ALPHA).
+                blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
+                         GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
+                glBindTexture(GL_TEXTURE_2D, p.tex_id);
+            }
+
+            glVertexAttribPointer(prog.position_attr, 3, GL_FLOAT,
+                                  GL_FALSE, sizeof(mgl::Vertex),
+                                  &p.vertices[0].position);
+            glVertexAttribPointer(prog.texcoord_attr, 2, GL_FLOAT,
+                                  GL_FALSE, sizeof(mgl::Vertex),
+                                  &p.vertices[0].texcoord);
+
+            if (blend.dst_rgb == GL_ZERO)
+            {
+                glDisable(GL_BLEND);
+            }
+            else
+            {
+                glEnable(GL_BLEND);
+                glBlendFuncSeparate(blend.src_rgb,   blend.dst_rgb,
+                                    blend.src_alpha, blend.dst_alpha);
+            }
+
+            glDrawArrays(p.type, 0, p.nvertices);
         }
-
-        glDrawArrays(p.type, 0, p.nvertices);
+    }
+    catch (std::exception const& ex)
+    {
+        report_exception();
     }
 
     glDisableVertexAttribArray(prog.texcoord_attr);