Mir

Merge lp:~alan-griffiths/mir/fix-1643446 into lp:mir

Proposed by Alan Griffiths on 2017-08-08
Status: Merged
Approved by: Alan Griffiths on 2017-08-10
Approved revision: 4224
Merged at revision: 4222
Proposed branch: lp:~alan-griffiths/mir/fix-1643446
Merge into: lp:mir
Diff against target: 60 lines (+17/-1)
2 files modified
src/server/scene/mediating_display_changer.cpp (+16/-0)
tests/unit-tests/scene/test_mediating_display_changer.cpp (+1/-1)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1643446
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) 2017-08-08 Approve on 2017-08-09
Mir CI Bot continuous-integration Approve on 2017-08-09
Review via email: mp+328730@code.launchpad.net

Commit message

Fix reporting and handling of invalid display configuration. (LP: #1643446)

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4221
https://mir-jenkins.ubuntu.com/job/mir-ci/3544/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4849/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5050
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5039
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5039
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5039
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4886/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4886/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4886/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4886/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4886/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4886
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4886/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4886
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4886/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4886/console

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

review: Needs Fixing (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

17:17:29 11: [ FAILED ] NestedServer.given_nested_server_set_base_display_configuration_when_monitor_plugged_in_configuration_is_reset (5902 ms)
17:41:03 26: [ FAILED ] DisplayTestGeneric.configure_disallows_invalid_configuration

How do those pass locally?!

lp:~alan-griffiths/mir/fix-1643446 updated on 2017-08-09
4222. By Alan Griffiths on 2017-08-09

Validate configurations in MediatingDisplayChanger before trying to apply them

4223. By Alan Griffiths on 2017-08-09

Revert first approach

4224. By Alan Griffiths on 2017-08-09

DRY

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4223
https://mir-jenkins.ubuntu.com/job/mir-ci/3545/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4851/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5052
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5041
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5041
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5041
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4888/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4888/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4888/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4888/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4888/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4888/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4888/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4888
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4888/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

15:24:51 [ FAILED ] 4 tests, listed below:
15:24:51 [ FAILED ] ServerDisconnect.is_detected_by_client
15:24:51 [ FAILED ] ServerDisconnect.doesnt_stop_client_calling_API_functions
15:24:51 [ FAILED ] ServerStartup.creates_endpoint_on_filesystem
15:24:51 [ FAILED ] ServerStartup.after_server_sigkilled_can_start_new_instance

That's odd!

I suspect these are "inherited" from:

15:13:13 11: [ RUN ] NestedServerWithTwoDisplays.uses_passthrough_when_surface_size_is_appropriate
...
15:13:18 11: ==2589== Thread 6 Mir/Input Reade:
15:13:18 11: ==2589== Invalid read of size 8
15:13:18 11: ==2589== at 0x41FFA8A: std::__shared_ptr<mir_test_framework::FakeInputDeviceImpl::InputDevice, (__gnu_cxx::_Lock_policy)2>::operator->() const (shared_ptr_base.h:1055)
15:13:18 11: ==2589== by 0x41FB664: mir_test_framework::FakeInputDeviceImpl::emit_event(mir::input::synthesis::KeyParameters const&)::{lambda()#1}::operator()() const (fake_input_device_impl.cpp:68)

And that's unrelated.

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4224
https://mir-jenkins.ubuntu.com/job/mir-ci/3546/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4852
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5053
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5042
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5042
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5042
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4889/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4889
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4889/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

lgtm and tests seem happy now

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/scene/mediating_display_changer.cpp'
2--- src/server/scene/mediating_display_changer.cpp 2017-07-28 17:00:43 +0000
3+++ src/server/scene/mediating_display_changer.cpp 2017-08-09 15:03:50 +0000
4@@ -42,6 +42,7 @@
5
6 namespace
7 {
8+char const* const bad_display_config = "Invalid or inconsistent display configuration";
9
10 class DisplayConfigurationInProgressError : public mir::ClientVisibleError
11 {
12@@ -184,6 +185,11 @@
13 std::shared_ptr<mf::Session> const& session,
14 std::shared_ptr<mg::DisplayConfiguration> const& conf)
15 {
16+ if (!conf->valid())
17+ {
18+ BOOST_THROW_EXCEPTION(std::runtime_error(bad_display_config));
19+ }
20+
21 {
22 std::lock_guard<std::mutex> lg{configuration_mutex};
23 config_map[session] = conf;
24@@ -262,6 +268,11 @@
25 std::shared_ptr<graphics::DisplayConfiguration> const& conf,
26 std::chrono::seconds timeout)
27 {
28+ if (!conf->valid())
29+ {
30+ BOOST_THROW_EXCEPTION(std::runtime_error(bad_display_config));
31+ }
32+
33 {
34 std::lock_guard<std::mutex> lock{configuration_mutex};
35
36@@ -553,6 +564,11 @@
37
38 void ms::MediatingDisplayChanger::set_base_configuration(std::shared_ptr<mg::DisplayConfiguration> const &conf)
39 {
40+ if (!conf->valid())
41+ {
42+ BOOST_THROW_EXCEPTION(std::runtime_error(bad_display_config));
43+ }
44+
45 server_action_queue->enqueue(
46 this,
47 [this, conf]
48
49=== modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
50--- tests/unit-tests/scene/test_mediating_display_changer.cpp 2017-07-28 17:00:43 +0000
51+++ tests/unit-tests/scene/test_mediating_display_changer.cpp 2017-08-09 15:03:50 +0000
52@@ -454,7 +454,7 @@
53 conf->for_each_output(
54 [&new_output_enabled](mg::UserDisplayConfigurationOutput& output)
55 {
56- if (!output.used)
57+ if (output.connected && !output.used)
58 {
59 new_output_enabled = true;
60 output.used = true;

Subscribers

People subscribed via source and target branches