Mir

Merge lp:~andreas-pokorny/mir/unify-unwinder into lp:mir

  1. unify-unwinder
  2. Merge into development-branch
Proposed by Andreas Pokorny
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 2727
Proposed branch: lp:~andreas-pokorny/mir/unify-unwinder
Merge into: lp:mir
Diff against target: 300 lines (+110/-87)
3 files modified
src/include/common/mir/unwind_helpers.h (+76/-0)
src/server/compositor/multi_threaded_compositor.cpp (+9/-35)
src/server/display_server.cpp (+25/-52)
To merge this branch: bzr merge lp:~andreas-pokorny/mir/unify-unwinder
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Robert Carr (community) Approve
Kevin DuBois (community) Approve
Review via email: mp+264042@code.launchpad.net

Commit message

Unifies ApplyIfUnwinding and TryButRevertIfUnwinding

The unified class template can be used with
   auto var = try_but_revert_if_unwinding(apply_function, revert_function);
or
   auto var = on_unwind(revert_function);

Description of the change

This change unifies two scope guards used to handle necessary clean-up steps in case of stack unwinding with a slightly leaner version.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2723
http://jenkins.qa.ubuntu.com/job/mir-ci/4270/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3089
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/610
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3037
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/419
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/419/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3037
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3037/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5847
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21695

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

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

try_but_revert_if_unwinding.h
unwind_helpers.h seems a less cumbersome to remember, but lgtm.

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2724
http://jenkins.qa.ubuntu.com/job/mir-ci/4275/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3096
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/617
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3044
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/424
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/424/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3044
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3044/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5854
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21705

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added file 'src/include/common/mir/unwind_helpers.h'
--- src/include/common/mir/unwind_helpers.h 1970-01-01 00:00:00 +0000
+++ src/include/common/mir/unwind_helpers.h 2015-07-07 21:13:58 +0000
@@ -0,0 +1,76 @@
1/*
2 * Copyright © 2015 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * it under the terms of the GNU Lesser General Public License version 3 as
6 * published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Andreas Pokorny <andreas.pokorny@canonical.com>
17 */
18
19#ifndef MIR_UNWIND_HELPERS_H_
20#define MIR_UNWIND_HELPERS_H_
21
22#include <stdexcept>
23#include <iostream>
24
25namespace mir
26{
27template<typename Unwind>
28class RevertIfUnwinding
29{
30public:
31 template<typename Apply>
32 RevertIfUnwinding(Apply && apply, Unwind&& unwind)
33 : unwind{std::move(unwind)}
34 {
35 apply();
36 }
37
38 RevertIfUnwinding(Unwind&& unwind)
39 : unwind{std::move(unwind)}
40 {
41 }
42
43 RevertIfUnwinding(RevertIfUnwinding<Unwind> && rhs)
44 : unwind{std::move(rhs.unwind)}
45 {
46 }
47
48 ~RevertIfUnwinding()
49 {
50 if (std::uncaught_exception())
51 unwind();
52 }
53
54private:
55 RevertIfUnwinding(RevertIfUnwinding const&) = delete;
56 RevertIfUnwinding& operator=(RevertIfUnwinding const&) = delete;
57
58 Unwind unwind;
59};
60
61template<typename Apply, typename Revert>
62inline auto try_but_revert_if_unwinding(Apply && apply, Revert && reverse) -> RevertIfUnwinding<Revert>
63{
64 return RevertIfUnwinding<Revert>{std::move(apply), std::move(reverse)};
65}
66
67
68template<typename Revert>
69inline auto on_unwind(Revert && action) -> RevertIfUnwinding<Revert>
70{
71 return RevertIfUnwinding<Revert>{std::move(action)};
72}
73
74}
75
76#endif
077
=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp 2015-06-26 08:54:18 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp 2015-07-07 21:13:58 +0000
@@ -29,6 +29,7 @@
29#include "mir/scene/surface.h"29#include "mir/scene/surface.h"
30#include "mir/terminate_with_current_exception.h"30#include "mir/terminate_with_current_exception.h"
31#include "mir/raii.h"31#include "mir/raii.h"
32#include "mir/unwind_helpers.h"
32#include "mir/thread_name.h"33#include "mir/thread_name.h"
3334
34#include <thread>35#include <thread>
@@ -42,32 +43,6 @@
42namespace mg = mir::graphics;43namespace mg = mir::graphics;
43namespace ms = mir::scene;44namespace ms = mir::scene;
4445
45namespace
46{
47
48class ApplyIfUnwinding
49{
50public:
51 ApplyIfUnwinding(std::function<void()> const& apply)
52 : apply{apply}
53 {
54 }
55
56 ~ApplyIfUnwinding()
57 {
58 if (std::uncaught_exception())
59 apply();
60 }
61
62private:
63 ApplyIfUnwinding(ApplyIfUnwinding const&) = delete;
64 ApplyIfUnwinding& operator=(ApplyIfUnwinding const&) = delete;
65
66 std::function<void()> const apply;
67};
68
69}
70
71namespace mir46namespace mir
72{47{
73namespace compositor48namespace compositor
@@ -118,12 +93,12 @@
118 void operator()() noexcept // noexcept is important! (LP: #1237332)93 void operator()() noexcept // noexcept is important! (LP: #1237332)
119 try94 try
120 {95 {
121 ApplyIfUnwinding on_startup_failure{96 auto on_startup_failure = on_unwind(
122 [this]97 [this]
123 {98 {
124 if (started_future.wait_for(0s) != std::future_status::ready)99 if (started_future.wait_for(0s) != std::future_status::ready)
125 started.set_exception(std::current_exception());100 started.set_exception(std::current_exception());
126 }};101 });
127102
128 mir::set_thread_name("Mir/Comp");103 mir::set_thread_name("Mir/Comp");
129104
@@ -311,10 +286,8 @@
311 report->started();286 report->started();
312287
313 /* To cleanup state if any code below throws */288 /* To cleanup state if any code below throws */
314 ApplyIfUnwinding cleanup_if_unwinding{289 auto cleanup_if_unwinding = on_unwind(
315 [this, &lk]{290 [this, &lk]{ destroy_compositing_threads(lk); });
316 destroy_compositing_threads(lk);
317 }};
318291
319 lk.unlock();292 lk.unlock();
320 scene->add_observer(observer);293 scene->add_observer(observer);
@@ -341,11 +314,12 @@
341 state = CompositorState::stopping;314 state = CompositorState::stopping;
342315
343 /* To cleanup state if any code below throws */316 /* To cleanup state if any code below throws */
344 ApplyIfUnwinding cleanup_if_unwinding{317 auto cleanup_if_unwinding = on_unwind(
345 [this, &lk]{318 [this, &lk]
319 {
346 if(!lk.owns_lock()) lk.lock();320 if(!lk.owns_lock()) lk.lock();
347 state = CompositorState::started;321 state = CompositorState::started;
348 }};322 });
349323
350 lk.unlock();324 lk.unlock();
351 scene->remove_observer(observer);325 scene->remove_observer(observer);
352326
=== modified file 'src/server/display_server.cpp'
--- src/server/display_server.cpp 2015-06-17 05:20:42 +0000
+++ src/server/display_server.cpp 2015-07-07 21:13:58 +0000
@@ -31,6 +31,7 @@
31#include "mir/input/input_manager.h"31#include "mir/input/input_manager.h"
32#include "mir/input/input_dispatcher.h"32#include "mir/input/input_dispatcher.h"
33#include "mir/log.h"33#include "mir/log.h"
34#include "mir/unwind_helpers.h"
3435
35#include <stdexcept>36#include <stdexcept>
3637
@@ -40,34 +41,6 @@
40namespace mi = mir::input;41namespace mi = mir::input;
41namespace msh = mir::shell;42namespace msh = mir::shell;
4243
43namespace
44{
45
46class TryButRevertIfUnwinding
47{
48public:
49 TryButRevertIfUnwinding(std::function<void()> const& apply,
50 std::function<void()> const& revert)
51 : revert{revert}
52 {
53 apply();
54 }
55
56 ~TryButRevertIfUnwinding()
57 {
58 if (std::uncaught_exception())
59 revert();
60 }
61
62private:
63 TryButRevertIfUnwinding(TryButRevertIfUnwinding const&) = delete;
64 TryButRevertIfUnwinding& operator=(TryButRevertIfUnwinding const&) = delete;
65
66 std::function<void()> const revert;
67};
68
69}
70
71struct mir::DisplayServer::Private44struct mir::DisplayServer::Private
72{45{
73 Private(ServerConfiguration& config)46 Private(ServerConfiguration& config)
@@ -97,29 +70,29 @@
97 {70 {
98 try71 try
99 {72 {
100 TryButRevertIfUnwinding comm{73 auto comm = try_but_revert_if_unwinding(
101 [this] { connector->stop(); },74 [this] { connector->stop(); },
102 [this] { connector->start(); }};75 [this] { connector->start(); });
10376
104 TryButRevertIfUnwinding prompt{77 auto prompt = try_but_revert_if_unwinding(
105 [this] { prompt_connector->stop(); },78 [this] { prompt_connector->stop(); },
106 [this] { prompt_connector->start(); }};79 [this] { prompt_connector->start(); });
10780
108 TryButRevertIfUnwinding dispatcher{81 auto dispatcher = try_but_revert_if_unwinding(
109 [this] { input_dispatcher->stop(); },82 [this] { input_dispatcher->stop(); },
110 [this] { input_dispatcher->start(); }};83 [this] { input_dispatcher->start(); });
11184
112 TryButRevertIfUnwinding input{85 auto input = try_but_revert_if_unwinding(
113 [this] { input_manager->stop(); },86 [this] { input_manager->stop(); },
114 [this] { input_manager->start(); }};87 [this] { input_manager->start(); });
11588
116 TryButRevertIfUnwinding display_config_processing{89 auto display_config_processing = try_but_revert_if_unwinding(
117 [this] { display_changer->pause_display_config_processing(); },90 [this] { display_changer->pause_display_config_processing(); },
118 [this] { display_changer->resume_display_config_processing(); }};91 [this] { display_changer->resume_display_config_processing(); });
11992
120 TryButRevertIfUnwinding comp{93 auto comp = try_but_revert_if_unwinding(
121 [this] { compositor->stop(); },94 [this] { compositor->stop(); },
122 [this] { compositor->start(); }};95 [this] { compositor->start(); });
12396
124 display->pause();97 display->pause();
125 }98 }
@@ -137,29 +110,29 @@
137 {110 {
138 try111 try
139 {112 {
140 TryButRevertIfUnwinding disp{113 auto disp = try_but_revert_if_unwinding(
141 [this] { display->resume(); },114 [this] { display->resume(); },
142 [this] { display->pause(); }};115 [this] { display->pause(); });
143116
144 TryButRevertIfUnwinding comp{117 auto comp = try_but_revert_if_unwinding(
145 [this] { compositor->start(); },118 [this] { compositor->start(); },
146 [this] { compositor->stop(); }};119 [this] { compositor->stop(); });
147120
148 TryButRevertIfUnwinding display_config_processing{121 auto display_config_processing = try_but_revert_if_unwinding(
149 [this] { display_changer->resume_display_config_processing(); },122 [this] { display_changer->resume_display_config_processing(); },
150 [this] { display_changer->pause_display_config_processing(); }};123 [this] { display_changer->pause_display_config_processing(); });
151124
152 TryButRevertIfUnwinding input{125 auto input = try_but_revert_if_unwinding(
153 [this] { input_manager->start(); },126 [this] { input_manager->start(); },
154 [this] { input_manager->stop(); }};127 [this] { input_manager->stop(); });
155128
156 TryButRevertIfUnwinding dispatcher{129 auto dispatcher = try_but_revert_if_unwinding(
157 [this] { input_dispatcher->start(); },130 [this] { input_dispatcher->start(); },
158 [this] { input_dispatcher->stop(); }};131 [this] { input_dispatcher->stop(); });
159132
160 TryButRevertIfUnwinding prompt{133 auto prompt = try_but_revert_if_unwinding(
161 [this] { prompt_connector->start(); },134 [this] { prompt_connector->start(); },
162 [this] { prompt_connector->stop(); }};135 [this] { prompt_connector->stop(); });
163136
164 connector->start();137 connector->start();
165 }138 }

Subscribers

People subscribed via source and target branches