Merge lp:~kuchtam/mir/Bug_1664760 into lp:mir

[Michał Kuchta]

Fix for server crash LP: #1664760

The main reason of this crash is to performing (in clean_onscreen_buffers function) operations on deque (erase) after which previous iterators and references can be invalidated.

I committed fix for bug: 1664760.

The main reason of this crash is to performing (in clean_onscreen_buffers function) operations on deque (erase) after which previous iterators and references can be invalidated.

[Daniel van Vugt (vanvugt)]

Great fix, thanks!

I think it would be more clear and safer for future maintenance though to save the return value before the iterator gets invalidated:

    auto& last_entry = onscreen_buffers.front();
    last_entry.use_count++;
    auto ret = last_entry.buffer;
    if (mode == mc::MultiMonitorMode::multi_monitor_sync)
        clean_onscreen_buffers(lk);
    return ret;

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

review: Approve

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

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1122/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4025/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1184/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4112
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4102
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4102
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4102
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4052
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4052/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/4052
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4052
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4052/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4052/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4052/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/4052
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4052/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/4052
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4052/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

^^^
Bug 1661187

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

review: Approve (continuous-integration)

=== modified file 'src/server/compositor/multi_monitor_arbiter.cpp'
--- src/server/compositor/multi_monitor_arbiter.cpp	2017-01-18 02:29:37 +0000
+++ src/server/compositor/multi_monitor_arbiter.cpp	2017-02-17 22:45:56 +0000
@@ -67,9 +67,10 @@
 
     auto& last_entry = onscreen_buffers.front();
     last_entry.use_count++;
+    auto last_entry_buffer = last_entry.buffer;
     if (mode == mc::MultiMonitorMode::multi_monitor_sync)
         clean_onscreen_buffers(lk);
-    return last_entry.buffer;
+    return last_entry_buffer;
 }
 
 void mc::MultiMonitorArbiter::compositor_release(std::shared_ptr<mg::Buffer> const& buffer)
 
=== modified file 'tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp'
--- tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp	2017-01-18 02:29:37 +0000
+++ tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp	2017-02-17 22:45:56 +0000
@@ -538,6 +538,23 @@
     EXPECT_THAT(b3->id(), Eq(buffers[2]->id()));
 }
 
+TEST_F(MultiMonitorArbiter, checks_if_buffer_is_valid_after_clean_onscreen_buffer)
+{
+    int comp_id1{0};
+
+    schedule.set_schedule({buffers[0], buffers[1], buffers[2], buffers[3]});
+
+    arbiter.advance_schedule();
+    arbiter.advance_schedule();
+    arbiter.advance_schedule();
+    arbiter.advance_schedule();
+
+    auto b1 = arbiter.compositor_acquire(&comp_id1);
+
+    EXPECT_THAT(b1->id(), Eq(buffers[3]->id()));
+    EXPECT_THAT(b1->size(), Eq(buffers[3]->size()));
+}
+
 TEST_F(MultiMonitorArbiter, releases_buffer_on_destruction)
 {
     mc::MultiMonitorArbiter arbiter{guarantee, mt::fake_shared(mock_map), mt::fake_shared(schedule)};