Merge lp:~cemil-azizoglu/mir/fix-1661498 into lp:mir

[Cemil Azizoglu]

Fix lp:1661498.

This is due to the DisplayConfiguration being a fake shared_ptr instead of a genuine shared_ptr. Very rarely, but very frequently on a silo, the test finishes and the object backing the fake pointer gets destroyed, while on a different thread, display config information is still tried to be logged off of this pointer.

This can be reproduced locally more regularly by making the test code faster (e.g. comment out the code after apply_config_change_and_wait_for_propagation()).

Indeed in many other tests in this file, a shared pointer is used for the config. So this change makes things more consistent as well.

With this fix, the silo is happy.

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

PASSED: Continuous integration, rev:4101
https://mir-jenkins.ubuntu.com/job/mir-ci/3207/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4311
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4398
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4388
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4388
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4388
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4343/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/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4343/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4343/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/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4343/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/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4343/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/4343
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4343/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

OK yeah. If we don't have explicit control over our threads then a proper shared_ptr is required.

Although this implies that the shared object should also be doing its own locking to be thread- safe...?

review: Approve

[Chris Halse Rogers (raof)]

I don't think this implies the object should be doing its own locking - the problem here is that it's being used after destruction. The Display holds locks when accessing the mutable methods on it.

LGTM.

review: Approve

=== modified file 'tests/acceptance-tests/test_new_display_configuration.cpp'
--- tests/acceptance-tests/test_new_display_configuration.cpp	2017-03-14 02:26:28 +0000
+++ tests/acceptance-tests/test_new_display_configuration.cpp	2017-03-21 22:29:34 +0000
@@ -475,9 +475,10 @@
     using namespace testing;
 
     auto format = GetParam();
-    mtd::StubDisplayConfig single_format_config(1, {format});
+    auto single_format_config =
+        std::make_shared<mtd::StubDisplayConfig>(1, std::vector<MirPixelFormat>{format});
 
-    apply_config_change_and_wait_for_propagation(mt::fake_shared(single_format_config));
+    apply_config_change_and_wait_for_propagation(single_format_config);
 
     DisplayClient client{new_connection()};
 
@@ -507,9 +508,11 @@
         60.0,
         true,
         subpixel_arrangement};
-    mtd::StubDisplayConfig single_subpixel_config({output});
-
-    apply_config_change_and_wait_for_propagation(mt::fake_shared(single_subpixel_config));
+
+    auto single_subpixel_config =
+        std::make_shared<mtd::StubDisplayConfig>(std::vector<mg::DisplayConfigurationOutput>{output});
+
+    apply_config_change_and_wait_for_propagation(single_subpixel_config);
 
     DisplayClient client{new_connection()};