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

[Andreas Pokorny]

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);

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

[PS Jenkins bot (ps-jenkins)]

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)

[Kevin DuBois (kdub)]

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

review: Approve

[Robert Carr (robertcarr)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

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)

=== 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 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andreas Pokorny <[e-mail]>
+ */
+
+#ifndef MIR_UNWIND_HELPERS_H_
+#define MIR_UNWIND_HELPERS_H_
+
+#include <stdexcept>
+#include <iostream>
+
+namespace mir
+{
+template<typename Unwind>
+class RevertIfUnwinding
+{
+public:
+    template<typename Apply>
+    RevertIfUnwinding(Apply && apply, Unwind&& unwind)
+        : unwind{std::move(unwind)}
+    {
+        apply();
+    }
+
+    RevertIfUnwinding(Unwind&& unwind)
+        : unwind{std::move(unwind)}
+    {
+    }
+
+    RevertIfUnwinding(RevertIfUnwinding<Unwind> && rhs)
+        : unwind{std::move(rhs.unwind)}
+    {
+    }
+
+    ~RevertIfUnwinding()
+    {
+        if (std::uncaught_exception())
+            unwind();
+    }
+
+private:
+    RevertIfUnwinding(RevertIfUnwinding const&) = delete;
+    RevertIfUnwinding& operator=(RevertIfUnwinding const&) = delete;
+
+    Unwind unwind;
+};
+
+template<typename Apply, typename Revert>
+inline auto try_but_revert_if_unwinding(Apply && apply, Revert && reverse) -> RevertIfUnwinding<Revert>
+{
+    return RevertIfUnwinding<Revert>{std::move(apply), std::move(reverse)};
+}
+
+
+template<typename Revert>
+inline auto on_unwind(Revert && action) -> RevertIfUnwinding<Revert>
+{
+    return RevertIfUnwinding<Revert>{std::move(action)};
+}
+
+}
+
+#endif

=== 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 @@
 #include "mir/scene/surface.h"
 #include "mir/terminate_with_current_exception.h"
 #include "mir/raii.h"
+#include "mir/unwind_helpers.h"
 #include "mir/thread_name.h"
 
 #include <thread>
@@ -42,32 +43,6 @@
 namespace mg = mir::graphics;
 namespace ms = mir::scene;
 
-namespace
-{
-
-class ApplyIfUnwinding
-{
-public:
-    ApplyIfUnwinding(std::function<void()> const& apply)
-        : apply{apply}
-    {
-    }
-
-    ~ApplyIfUnwinding()
-    {
-        if (std::uncaught_exception())
-            apply();
-    }
-
-private:
-    ApplyIfUnwinding(ApplyIfUnwinding const&) = delete;
-    ApplyIfUnwinding& operator=(ApplyIfUnwinding const&) = delete;
-
-    std::function<void()> const apply;
-};
-
-}
-
 namespace mir
 {
 namespace compositor
@@ -118,12 +93,12 @@
     void operator()() noexcept  // noexcept is important! (LP: #1237332)
     try
     {
-        ApplyIfUnwinding on_startup_failure{
+        auto on_startup_failure = on_unwind(
             [this]
             {
                 if (started_future.wait_for(0s) != std::future_status::ready)
                     started.set_exception(std::current_exception());
-            }};
+            });
 
         mir::set_thread_name("Mir/Comp");
 
@@ -311,10 +286,8 @@
     report->started();
 
     /* To cleanup state if any code below throws */
-    ApplyIfUnwinding cleanup_if_unwinding{
-        [this, &lk]{
-            destroy_compositing_threads(lk);
-        }};
+    auto cleanup_if_unwinding = on_unwind(
+        [this, &lk]{ destroy_compositing_threads(lk); });
 
     lk.unlock();
     scene->add_observer(observer);
@@ -341,11 +314,12 @@
     state = CompositorState::stopping;
 
     /* To cleanup state if any code below throws */
-    ApplyIfUnwinding cleanup_if_unwinding{
-        [this, &lk]{
+    auto cleanup_if_unwinding = on_unwind(
+        [this, &lk]
+        {
             if(!lk.owns_lock()) lk.lock();
             state = CompositorState::started;
-        }};
+        });
 
     lk.unlock();
     scene->remove_observer(observer);

=== 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 @@
 #include "mir/input/input_manager.h"
 #include "mir/input/input_dispatcher.h"
 #include "mir/log.h"
+#include "mir/unwind_helpers.h"
 
 #include <stdexcept>
 
@@ -40,34 +41,6 @@
 namespace mi = mir::input;
 namespace msh = mir::shell;
 
-namespace
-{
-
-class TryButRevertIfUnwinding
-{
-public:
-    TryButRevertIfUnwinding(std::function<void()> const& apply,
-                            std::function<void()> const& revert)
-        : revert{revert}
-    {
-        apply();
-    }
-
-    ~TryButRevertIfUnwinding()
-    {
-        if (std::uncaught_exception())
-            revert();
-    }
-
-private:
-    TryButRevertIfUnwinding(TryButRevertIfUnwinding const&) = delete;
-    TryButRevertIfUnwinding& operator=(TryButRevertIfUnwinding const&) = delete;
-
-    std::function<void()> const revert;
-};
-
-}
-
 struct mir::DisplayServer::Private
 {
     Private(ServerConfiguration& config)
@@ -97,29 +70,29 @@
     {
         try
         {
-            TryButRevertIfUnwinding comm{
+            auto comm = try_but_revert_if_unwinding(
                 [this] { connector->stop(); },
-                [this] { connector->start(); }};
+                [this] { connector->start(); });
 
-            TryButRevertIfUnwinding prompt{
+            auto prompt = try_but_revert_if_unwinding(
                 [this] { prompt_connector->stop(); },
-                [this] { prompt_connector->start(); }};
+                [this] { prompt_connector->start(); });
 
-            TryButRevertIfUnwinding dispatcher{
+            auto dispatcher = try_but_revert_if_unwinding(
                 [this] { input_dispatcher->stop(); },
-                [this] { input_dispatcher->start(); }};
+                [this] { input_dispatcher->start(); });
 
-            TryButRevertIfUnwinding input{
+            auto input = try_but_revert_if_unwinding(
                 [this] { input_manager->stop(); },
-                [this] { input_manager->start(); }};
+                [this] { input_manager->start(); });
 
-            TryButRevertIfUnwinding display_config_processing{
+            auto display_config_processing = try_but_revert_if_unwinding(
                 [this] { display_changer->pause_display_config_processing(); },
-                [this] { display_changer->resume_display_config_processing(); }};
+                [this] { display_changer->resume_display_config_processing(); });
 
-            TryButRevertIfUnwinding comp{
+            auto comp = try_but_revert_if_unwinding(
                 [this] { compositor->stop(); },
-                [this] { compositor->start(); }};
+                [this] { compositor->start(); });
 
             display->pause();
         }
@@ -137,29 +110,29 @@
     {
         try
         {
-            TryButRevertIfUnwinding disp{
+            auto disp = try_but_revert_if_unwinding(
                 [this] { display->resume(); },
-                [this] { display->pause(); }};
+                [this] { display->pause(); });
 
-            TryButRevertIfUnwinding comp{
+            auto comp = try_but_revert_if_unwinding(
                 [this] { compositor->start(); },
-                [this] { compositor->stop(); }};
+                [this] { compositor->stop(); });
 
-            TryButRevertIfUnwinding display_config_processing{
+            auto display_config_processing = try_but_revert_if_unwinding(
                 [this] { display_changer->resume_display_config_processing(); },
-                [this] { display_changer->pause_display_config_processing(); }};
+                [this] { display_changer->pause_display_config_processing(); });
 
-            TryButRevertIfUnwinding input{
+            auto input = try_but_revert_if_unwinding(
                 [this] { input_manager->start(); },
-                [this] { input_manager->stop(); }};
+                [this] { input_manager->stop(); });
 
-            TryButRevertIfUnwinding dispatcher{
+            auto dispatcher = try_but_revert_if_unwinding(
                 [this] { input_dispatcher->start(); },
-                [this] { input_dispatcher->stop(); }};
+                [this] { input_dispatcher->stop(); });
 
-            TryButRevertIfUnwinding prompt{
+            auto prompt = try_but_revert_if_unwinding(
                 [this] { prompt_connector->start(); },
-                [this] { prompt_connector->stop(); }};
+                [this] { prompt_connector->stop(); });
 
             connector->start();
         }