Merge lp:~alan-griffiths/mir/bfi-fix-1416482 into lp:mir/0.11

[Alan Griffiths]

compositor: avoid segfault when running two monitors "sidebyside"

compositor: avoid segfault when running two monitors "sidebyside"

I'm not really familiar with this area of code, so if there's a more elegant solution please suggest it.

[Alexandros Frantzis (afrantzis)]

We need to serialize one level higher, in GLProgramFamily::add_program(), to ensure glCreateProgram() is also protected from concurrent access.

Note that we now have two points at which we allocate program and shader ids: GLProgramFamily
and GLProgramFactory (used only in the Android backend after a recent refactoring). Although they may be (or rather will be after this fix) invidually protected from concurrent access, in theory two threads could run concurrently in the two different classes. Not sure if it's a problem in practice, but ideally the two classes should also get synchronized or merged (not in this MP); we don't really need two implementations offering the same functionality.

Some background:

This was first addressed in r923 [1] :

/*
 * We need to serialize renderer creation because some GL calls used
 * during renderer construction that create unique resource ids
 * (e.g. glCreateProgram) are not thread-safe when the threads are
 * have the same or shared EGL contexts.
 */

Since then the code has been refactored multiple times and the serialization logic moved around. In a recent refactoring (the transition from GLProgramFactory to GLProgramFamily for GLRenderer), the serialization logic was lost.

[1] https://bazaar.launchpad.net/~mir-team/mir/0.11/revision/923

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2283
http://jenkins.qa.ubuntu.com/job/mir-ci/2870/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1148
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1109
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/867
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/867/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1109
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1109/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4162
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17661

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2870/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2284
http://jenkins.qa.ubuntu.com/job/mir-ci/2872/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1150
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1111
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/869
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/869/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1111
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1111/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4164
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17665

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2872/rebuild

review: Approve (continuous-integration)

=== modified file 'src/server/compositor/gl_program_family.cpp'
--- src/server/compositor/gl_program_family.cpp	2015-01-07 03:18:31 +0000
+++ src/server/compositor/gl_program_family.cpp	2015-02-02 14:04:44 +0000
@@ -18,6 +18,8 @@
 
 #include "mir/compositor/gl_program_family.h"
 
+#include <mutex>
+
 namespace mir { namespace compositor {
 
 void GLProgramFamily::Shader::init(GLenum type, const GLchar* src)
@@ -73,6 +75,14 @@
 GLuint GLProgramFamily::add_program(const GLchar* const vshader_src,
                                     const GLchar* const fshader_src)
 {
+    // Serialize calls - avoids segfault on multimonitor (see lp:1416482)
+    // We need to serialize renderer creation because some GL calls used
+    // during renderer construction that create unique resource ids
+    // (e.g. glCreateProgram) are not thread-safe when the threads are
+    // have the same or shared EGL contexts.
+    static std::mutex lp1416482_mutex;
+    std::lock_guard<decltype(lp1416482_mutex)> lock{lp1416482_mutex};
+
     auto& v = vshader[vshader_src];
     if (!v.id) v.init(GL_VERTEX_SHADER, vshader_src);