Mir

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

Proposed by Alan Griffiths
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: 3732
Merged at revision: 3737
Proposed branch: lp:~alan-griffiths/mir/fix-server-crash
Merge into: lp:mir
Diff against target: 156 lines (+70/-61)
1 file modified
src/renderers/gl/renderer.cpp (+70/-61)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-server-crash
Reviewer Review Type Date Requested Status
Chris Halse Rogers Approve
Mir CI Bot continuous-integration Approve
Andreas Pokorny (community) Approve
Cemil Azizoglu (community) Approve
Review via email: mp+307459@code.launchpad.net

Commit message

Fix the server crash observed in lp:1629275

To post a comment you must log in.
3732. By Alan Griffiths

Tidy headers

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

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)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

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)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

A test case might be good.

review: Approve
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

ok

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

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)
Revision history for this message
Chris Halse Rogers (raof) wrote :

This looks sensible.

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

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

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 +44 (0)798 9938 758
Octopull Ltd http://www.octopull.co.uk/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/renderers/gl/renderer.cpp'
2--- src/renderers/gl/renderer.cpp 2016-09-19 04:16:15 +0000
3+++ src/renderers/gl/renderer.cpp 2016-10-03 16:14:19 +0000
4@@ -1,4 +1,4 @@
5-/* Copyright © 2013 Canonical Ltd.
6+/* Copyright © 2013, 2016 Canonical Ltd.
7 *
8 * This program is free software: you can redistribute it and/or modify it
9 * under the terms of the GNU General Public License version 3,
10@@ -28,6 +28,7 @@
11 #include "mir/gl/texture_cache.h"
12 #include "mir/gl/texture.h"
13 #include "mir/log.h"
14+#include "mir/report_exception.h"
15
16 #define GLM_FORCE_RADIANS
17 #include <glm/gtc/matrix_transform.hpp>
18@@ -287,70 +288,78 @@
19 primitives.clear();
20 tessellate(primitives, renderable);
21
22- auto surface_tex = texture_cache->load(renderable);
23-
24- typedef struct // Represents parameters of glBlendFuncSeparate()
25- {
26- GLenum src_rgb, dst_rgb, src_alpha, dst_alpha;
27- } BlendSeparate;
28-
29- BlendSeparate client_blend;
30-
31- // These renderable method names could be better (see LP: #1236224)
32- if (renderable.shaped()) // Client is RGBA:
33- {
34- client_blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
35- GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
36- }
37- else if (renderable.alpha() == 1.0f) // RGBX and no window translucency:
38- {
39- client_blend = {GL_ONE, GL_ZERO,
40- GL_ZERO, GL_ONE}; // Avoid using src_alpha!
41- }
42- else
43- { // Client is RGBX but we also have window translucency.
44- // The texture alpha channel is possibly uninitialized so we must be
45- // careful and avoid using SRC_ALPHA (LP: #1423462).
46- client_blend = {GL_ONE, GL_ONE_MINUS_CONSTANT_ALPHA,
47- GL_ZERO, GL_ONE};
48- glBlendColor(0.0f, 0.0f, 0.0f, renderable.alpha());
49- }
50-
51- for (auto const& p : primitives)
52- {
53- BlendSeparate blend;
54-
55- if (p.tex_id == 0) // The client surface texture
56- {
57- blend = client_blend;
58- surface_tex->bind();
59- }
60- else // Some other texture from the shell (e.g. decorations) which
61- { // is always RGBA (valid SRC_ALPHA).
62- blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
63- GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
64- glBindTexture(GL_TEXTURE_2D, p.tex_id);
65- }
66-
67- glVertexAttribPointer(prog.position_attr, 3, GL_FLOAT,
68- GL_FALSE, sizeof(mgl::Vertex),
69- &p.vertices[0].position);
70- glVertexAttribPointer(prog.texcoord_attr, 2, GL_FLOAT,
71- GL_FALSE, sizeof(mgl::Vertex),
72- &p.vertices[0].texcoord);
73-
74- if (blend.dst_rgb == GL_ZERO)
75- {
76- glDisable(GL_BLEND);
77+ // if we fail to load the texture, we need to carry on (part of lp:1629275)
78+ try
79+ {
80+ auto surface_tex = texture_cache->load(renderable);
81+
82+ typedef struct // Represents parameters of glBlendFuncSeparate()
83+ {
84+ GLenum src_rgb, dst_rgb, src_alpha, dst_alpha;
85+ } BlendSeparate;
86+
87+ BlendSeparate client_blend;
88+
89+ // These renderable method names could be better (see LP: #1236224)
90+ if (renderable.shaped()) // Client is RGBA:
91+ {
92+ client_blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
93+ GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
94+ }
95+ else if (renderable.alpha() == 1.0f) // RGBX and no window translucency:
96+ {
97+ client_blend = {GL_ONE, GL_ZERO,
98+ GL_ZERO, GL_ONE}; // Avoid using src_alpha!
99 }
100 else
101+ { // Client is RGBX but we also have window translucency.
102+ // The texture alpha channel is possibly uninitialized so we must be
103+ // careful and avoid using SRC_ALPHA (LP: #1423462).
104+ client_blend = {GL_ONE, GL_ONE_MINUS_CONSTANT_ALPHA,
105+ GL_ZERO, GL_ONE};
106+ glBlendColor(0.0f, 0.0f, 0.0f, renderable.alpha());
107+ }
108+
109+ for (auto const& p : primitives)
110 {
111- glEnable(GL_BLEND);
112- glBlendFuncSeparate(blend.src_rgb, blend.dst_rgb,
113- blend.src_alpha, blend.dst_alpha);
114+ BlendSeparate blend;
115+
116+ if (p.tex_id == 0) // The client surface texture
117+ {
118+ blend = client_blend;
119+ surface_tex->bind();
120+ }
121+ else // Some other texture from the shell (e.g. decorations) which
122+ { // is always RGBA (valid SRC_ALPHA).
123+ blend = {GL_ONE, GL_ONE_MINUS_SRC_ALPHA,
124+ GL_ONE, GL_ONE_MINUS_SRC_ALPHA};
125+ glBindTexture(GL_TEXTURE_2D, p.tex_id);
126+ }
127+
128+ glVertexAttribPointer(prog.position_attr, 3, GL_FLOAT,
129+ GL_FALSE, sizeof(mgl::Vertex),
130+ &p.vertices[0].position);
131+ glVertexAttribPointer(prog.texcoord_attr, 2, GL_FLOAT,
132+ GL_FALSE, sizeof(mgl::Vertex),
133+ &p.vertices[0].texcoord);
134+
135+ if (blend.dst_rgb == GL_ZERO)
136+ {
137+ glDisable(GL_BLEND);
138+ }
139+ else
140+ {
141+ glEnable(GL_BLEND);
142+ glBlendFuncSeparate(blend.src_rgb, blend.dst_rgb,
143+ blend.src_alpha, blend.dst_alpha);
144+ }
145+
146+ glDrawArrays(p.type, 0, p.nvertices);
147 }
148-
149- glDrawArrays(p.type, 0, p.nvertices);
150+ }
151+ catch (std::exception const& ex)
152+ {
153+ report_exception();
154 }
155
156 glDisableVertexAttribArray(prog.texcoord_attr);

Subscribers

People subscribed via source and target branches