Mir

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

Proposed by Michał Kuchta on 2017-02-15
Status: Merged
Approved by: Daniel van Vugt on 2017-02-20
Approved revision: 4033
Merged at revision: 4043
Proposed branch: lp:~kuchtam/mir/Bug_1664760
Merge into: lp:mir
Diff against target: 43 lines (+19/-1)
2 files modified
src/server/compositor/multi_monitor_arbiter.cpp (+2/-1)
tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp (+17/-0)
To merge this branch: bzr merge lp:~kuchtam/mir/Bug_1664760
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve on 2017-02-20
Daniel van Vugt 2017-02-15 Approve on 2017-02-19
Review via email: mp+317377@code.launchpad.net

Commit message

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.

Description of the change

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.

To post a comment you must log in.
Daniel van Vugt (vanvugt) wrote :

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) wrote :

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) wrote :
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/compositor/multi_monitor_arbiter.cpp'
2--- src/server/compositor/multi_monitor_arbiter.cpp 2017-01-18 02:29:37 +0000
3+++ src/server/compositor/multi_monitor_arbiter.cpp 2017-02-17 22:45:56 +0000
4@@ -67,9 +67,10 @@
5
6 auto& last_entry = onscreen_buffers.front();
7 last_entry.use_count++;
8+ auto last_entry_buffer = last_entry.buffer;
9 if (mode == mc::MultiMonitorMode::multi_monitor_sync)
10 clean_onscreen_buffers(lk);
11- return last_entry.buffer;
12+ return last_entry_buffer;
13 }
14
15 void mc::MultiMonitorArbiter::compositor_release(std::shared_ptr<mg::Buffer> const& buffer)
16
17=== modified file 'tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp'
18--- tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp 2017-01-18 02:29:37 +0000
19+++ tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp 2017-02-17 22:45:56 +0000
20@@ -538,6 +538,23 @@
21 EXPECT_THAT(b3->id(), Eq(buffers[2]->id()));
22 }
23
24+TEST_F(MultiMonitorArbiter, checks_if_buffer_is_valid_after_clean_onscreen_buffer)
25+{
26+ int comp_id1{0};
27+
28+ schedule.set_schedule({buffers[0], buffers[1], buffers[2], buffers[3]});
29+
30+ arbiter.advance_schedule();
31+ arbiter.advance_schedule();
32+ arbiter.advance_schedule();
33+ arbiter.advance_schedule();
34+
35+ auto b1 = arbiter.compositor_acquire(&comp_id1);
36+
37+ EXPECT_THAT(b1->id(), Eq(buffers[3]->id()));
38+ EXPECT_THAT(b1->size(), Eq(buffers[3]->size()));
39+}
40+
41 TEST_F(MultiMonitorArbiter, releases_buffer_on_destruction)
42 {
43 mc::MultiMonitorArbiter arbiter{guarantee, mt::fake_shared(mock_map), mt::fake_shared(schedule)};

Subscribers

People subscribed via source and target branches