Merge lp:~alan-griffiths/mir/raii-fix-1237710 into lp:mir

[Alan Griffiths]

run_mir: restore signals handlers after the server exits (eve if by an exception).

run_mir: restore signals handlers after the server exits (eve if by an exception).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1139
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/183/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2391
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2276
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/180
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/180/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/183/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

lgtm, i like fix-1237710 a bit more for the simplicity, but would be okay if this lands

review: Approve

[Kevin DuBois (kdub)]

I'll switch to abstain (would prefer lp:~alan-griffiths/mir/fix-1237710)

review: Abstain

[Alexandros Frantzis (afrantzis)]

I also like fix-1237710 for its simplicity, but I like uncoditional correctness more than simplicity :)

review: Approve

[Robert Carr (robertcarr)]

I feel like a contestant on a merge proposal game show.

Anyhow +1

review: Approve

[Daniel van Vugt (vanvugt)]

I appreciate the robustness you're aiming for here, but this approach is very difficult to read and maintain.

I'm going to approve the simplified version. And we can work on exception correctness after that. Maybe with a try/catch.

review: Disapprove

Unmerged revisions

1139. By Alan Griffiths on 2013-10-16

Restore previous signal handlers after run()

=== modified file 'src/server/run_mir.cpp'
--- src/server/run_mir.cpp	2013-10-14 06:33:42 +0000
+++ src/server/run_mir.cpp	2013-10-16 14:08:02 +0000
@@ -50,6 +50,16 @@
     signal(sig, old_handler[sig]);
     raise(sig);
 }
+
+struct PairedOperations
+{
+    PairedOperations(void (*set_up)(), void (*tear_down)()) :
+        tear_down(tear_down) { set_up(); }
+
+    ~PairedOperations() { tear_down(); }
+
+    void (* const tear_down)();
+};
 }
 
 void mir::run_mir(ServerConfiguration& config, std::function<void(DisplayServer&)> init)
@@ -71,8 +81,11 @@
 
     weak_connector = config.the_connector();
 
-    for (auto sig : { SIGQUIT, SIGABRT, SIGFPE, SIGSEGV, SIGBUS })
-        old_handler[sig] = signal(sig, fatal_signal_cleanup);
+    PairedOperations scoped(
+        [] { for (auto sig : { SIGQUIT, SIGABRT, SIGFPE, SIGSEGV, SIGBUS })
+                old_handler[sig] = signal(sig, fatal_signal_cleanup); },
+        [] { for (auto sig : { SIGQUIT, SIGABRT, SIGFPE, SIGSEGV, SIGBUS })
+                signal(sig, old_handler[sig]); });
 
     std::atexit(&delete_endpoint);