Merge lp:~afrantzis/mir/fix-1353867-sigterm-dispatch into lp:mir

[Alexandros Frantzis]

client: Fix SIGTERM dispatch in our default lifecycle event handler

Our default lifecycle event handler used to call raise() to send a TERM signal to the app when a connection break was detected. However, in a multi-threaded program raise() sends the signal to the current thread only. If the signal is blocked in that thread, then the signal is lost.

Depending on thread in which we detected that the connection was lost, the TERM signal would be dispatched (e.g. in the context of a driver callback asking for the next buffer) or not (e.g. in the context of our main RPC threads which block all signals). This MP fixes the issue by explicitly sending the signal to the process instead of the current thread. This ensures that it will be dispatched to some thread that doesn't block it.

client: Fix SIGTERM dispatch in our default lifecycle event handler

Our default lifecycle event handler used to call raise() to send a TERM signal to the app when a connection break was detected. However, in a multi-threaded program raise() sends the signal to the current thread only. If the signal is blocked in that thread, then the signal is lost.

Depending on thread in which we detected that the connection was lost, the TERM signal would be dispatched (e.g. in the context of a driver callback asking for the next buffer) or not (e.g. in the context of our main RPC threads which block all signals). This MP fixes the issue by explicitly sending the signal to the process instead of the current thread. This ensures that it will be dispatched to some thread that doesn't block it.

It turns out that we send a lifecycle connection lost event to ourselves when we release a client connection. With the signal dispatch issue fixed, this always leads to a SIGTERM being sent to the process when we call mir_connection_release() (with the default handler). This breaks most of the tests, since clients are expected to exit properly, not be terminated by a signal.

I assumed that sending a lifecycle event on release is a behavior we want, i.e. it's not accidental. If it's indeed accidental, we can remove it, along with the workaround in this MP.

The workaround proposed in this MP is to clear any lifecycle handlers just prior to disconnecting. This has the benefit that the default handler is active for the lifetime of the client and can therefore catch premature disconnections. It's not ideal, but it's reasonable for now (IMO).

As part of the workaround I refactored some of the test code to use newer infrastructure (like BasicClientServerFixture). However these refactorings are beneficial on their own and can remain even if we decide not to use the proposed workaround.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1837
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2366/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1286
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1292
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1274
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/888
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/888/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/889
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/889/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/210
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/210/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2372
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11327

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2366/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Manual testing OK. Verified bug 1353867 is fixed.

I suggest SIGPIPE or SIGHUP would be more appropriate than SIGTERM though. SIGTERM means the user has explicitly asked for the app to be killed, and that's not true in this case. It's more appropriate to use SIGPIPE or SIGHUP indicating the user did not explicitly kill the app, but we still want a clean termination (no core file).

review: Approve

[Chris Halse Rogers (raof)]

Looks sensible to me

review: Approve

[Daniel van Vugt (vanvugt)]

Essentially Approved still. Just please consider using a more appropriate signal: SIGPIPE or SIGHUP.

review: Needs Information

[Alexandros Frantzis (afrantzis)]

> Essentially Approved still. Just please consider using a more appropriate signal: SIGPIPE or SIGHUP.

I am not opposed to changing the signal, after some discussion about which one is best. A change to the signal needs to be followed by changes to signal handlers of our example clients so they can shut down properly. I'd rather we did all this in a separate MP and leave the current MP focused on just fixing the dispatch mechanism.

[Alan Griffiths (alan-griffiths)]

> > Essentially Approved still. Just please consider using a more appropriate
> signal: SIGPIPE or SIGHUP.
>
> I am not opposed to changing the signal, after some discussion about which one
> is best. A change to the signal needs to be followed by changes to signal
> handlers of our example clients so they can shut down properly. I'd rather we
> did all this in a separate MP and leave the current MP focused on just fixing
> the dispatch mechanism.

OK

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/986/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1353/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1359/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1336/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/370/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/370/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/273/console

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Happy to revisit the choice of signal later. One minor annoyance is stopping this from landing (per Jenkins):

Text conflict in tests/acceptance-tests/test_nested_mir.cpp
1 conflicts encountered.

review: Approve

=== modified file 'include/test/mir_test_framework/basic_client_server_fixture.h'
--- include/test/mir_test_framework/basic_client_server_fixture.h	2014-07-02 06:29:24 +0000
+++ include/test/mir_test_framework/basic_client_server_fixture.h	2014-08-12 10:44:30 +0000
@@ -22,6 +22,7 @@
 #include "mir_toolkit/mir_client_library.h"
 
 #include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 namespace mir_test_framework
 {
@@ -31,6 +32,7 @@
 struct BasicClientServerFixture : InProcessServer
 {
     TestServerConfiguration server_configuration;
+    UsingStubClientPlatform using_stub_client_platform;
 
     mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
 

=== modified file 'include/test/mir_test_framework/display_server_test_fixture.h'
--- include/test/mir_test_framework/display_server_test_fixture.h	2014-04-10 22:23:27 +0000
+++ include/test/mir_test_framework/display_server_test_fixture.h	2014-08-12 10:44:30 +0000
@@ -26,6 +26,7 @@
 
 #include <memory>
 #include <functional>
+#include <vector>
 
 namespace mir_test_framework
 {
@@ -65,6 +66,7 @@
 
     bool shutdown_server_process();
     Result  wait_for_shutdown_server_process();
+    std::vector<Result> wait_for_shutdown_client_processes();
     bool kill_server_process();
     void kill_client_processes();
     void terminate_client_processes();

=== modified file 'include/test/mir_test_framework/testing_process_manager.h'
--- include/test/mir_test_framework/testing_process_manager.h	2014-04-10 22:23:27 +0000
+++ include/test/mir_test_framework/testing_process_manager.h	2014-08-12 10:44:30 +0000
@@ -27,7 +27,7 @@
 #include "mir_test_framework/testing_client_configuration.h"
 
 #include <memory>
-#include <list>
+#include <vector>
 #include <functional>
 
 namespace mir
@@ -60,13 +60,14 @@
     Result shutdown_server_process();
     Result kill_server_process();
     Result wait_for_shutdown_server_process();
+    std::vector<Result> wait_for_shutdown_client_processes();
     void kill_client_processes();
     void terminate_client_processes();
     void run_in_test_process(std::function<void()> const& run_code);
 
 private:
     std::shared_ptr<Process> server_process;
-    std::list<std::shared_ptr<Process>> clients;
+    std::vector<std::shared_ptr<Process>> clients;
 
     bool is_test_process;
     bool server_process_was_started;

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2014-07-30 15:25:54 +0000
+++ src/client/mir_connection.cpp	2014-08-12 10:44:30 +0000
@@ -241,7 +241,12 @@
 {
     if (transition == mir_lifecycle_connection_lost)
     {
-        raise(SIGTERM);
+        /*
+         * We need to use kill() instead of raise() to ensure the signal
+         * is dispatched to the process even if it's blocked in the current
+         * thread.
+         */
+        kill(getpid(), SIGTERM);
     }
 }
 }

=== modified file 'tests/acceptance-tests/test_client_cursor_api.cpp'
--- tests/acceptance-tests/test_client_cursor_api.cpp	2014-07-10 03:45:37 +0000
+++ tests/acceptance-tests/test_client_cursor_api.cpp	2014-08-12 10:44:30 +0000
@@ -34,6 +34,7 @@
 #include "mir_test_framework/testing_client_configuration.h"
 #include "mir_test_framework/declarative_placement_strategy.h"
 #include "mir_test_framework/deferred_in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -234,6 +235,7 @@
     
     ServerConfiguration server_configuration{cursor_configured_fence, client_may_exit_fence};
     mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
+    mtf::UsingStubClientPlatform using_stub_client_platform;
     
     ClientConfig client_config_1{client_name_1, cursor_configured_fence, client_may_exit_fence};
     ClientConfig client_config_2{client_name_2, cursor_configured_fence, client_may_exit_fence};

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2014-08-06 01:40:20 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2014-08-12 10:44:30 +0000
@@ -32,6 +32,7 @@
 #include "mir_test_framework/input_testing_server_configuration.h"
 #include "mir_test_framework/input_testing_client_configuration.h"
 #include "mir_test_framework/declarative_placement_strategy.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -120,6 +121,7 @@
     mt::Barrier fence{2};
     mt::WaitCondition second_client_done;
     ServerConfiguration server_configuration{fence};
+    mtf::UsingStubClientPlatform using_stub_client_platform;
 
     mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
 

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2014-07-21 03:35:31 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2014-08-12 10:44:30 +0000
@@ -21,6 +21,7 @@
 
 #include "mir_test_framework/stubbed_server_configuration.h"
 #include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include "src/client/client_buffer.h"
 
@@ -51,13 +52,15 @@
 namespace mf = mir::frontend;
 namespace mc = mir::compositor;
 namespace mcl = mir::client;
+namespace mtf = mir_test_framework;
 
 namespace
 {
 struct ClientLibrary : mir_test_framework::InProcessServer
 {
-    mir_test_framework::StubbedServerConfiguration server_configuration;
+    mtf::StubbedServerConfiguration server_configuration;
     mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
+    mtf::UsingStubClientPlatform using_stub_client_platform;
 
     std::set<MirSurface*> surfaces;
     MirConnection* connection = nullptr;

=== modified file 'tests/acceptance-tests/test_client_screencast.cpp'
--- tests/acceptance-tests/test_client_screencast.cpp	2014-07-02 06:29:24 +0000
+++ tests/acceptance-tests/test_client_screencast.cpp	2014-08-12 10:44:30 +0000
@@ -22,8 +22,7 @@
 #include "mir_toolkit/mir_client_library.h"
 
 #include "mir_test_framework/stubbed_server_configuration.h"
-#include "mir_test_framework/in_process_server.h"
-#include "mir_test_framework/using_stub_client_platform.h"
+#include "mir_test_framework/basic_client_server_fixture.h"
 
 #include "mir_test_doubles/null_display_changer.h"
 #include "mir_test_doubles/stub_display_configuration.h"
@@ -84,15 +83,10 @@
     mtd::MockScreencast mock_screencast;
 };
 
-class MirScreencastTest : public mir_test_framework::InProcessServer
+struct MirScreencastTest : mtf::BasicClientServerFixture<StubServerConfig>
 {
-public:
-    mir::DefaultServerConfiguration& server_config() override { return server_config_; }
-
-    mtd::MockScreencast& mock_screencast() { return server_config_.mock_screencast; }
-
-    StubServerConfig server_config_;
-    mtf::UsingStubClientPlatform using_stub_client_platform;
+    mtd::MockScreencast& mock_screencast() { return server_configuration.mock_screencast; }
+
     MirScreencastParameters default_screencast_params {
         {0, 0, 1, 1}, 1, 1, mir_pixel_format_abgr_8888};
 };
@@ -111,9 +105,6 @@
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     mf::ScreencastSessionId const screencast_session_id{99};
 
     InSequence seq;
@@ -130,17 +121,12 @@
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
     ASSERT_NE(nullptr, screencast);
     mir_screencast_release_sync(screencast);
-
-    mir_connection_release(connection);
 }
 
 TEST_F(MirScreencastTest, contacts_server_screencast_with_provided_params)
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     mf::ScreencastSessionId const screencast_session_id{99};
 
     geom::Size const size{default_screencast_params.width, default_screencast_params.height};
@@ -164,17 +150,12 @@
     ASSERT_NE(nullptr, screencast);
 
     mir_screencast_release_sync(screencast);
-
-    mir_connection_release(connection);
 }
 
 TEST_F(MirScreencastTest, creation_with_invalid_params_fails)
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     MirScreencastParameters params = default_screencast_params;
     params.width = params.height = 0;
     auto screencast = mir_connection_create_screencast_sync(connection, &params);
@@ -190,17 +171,12 @@
 
     screencast = mir_connection_create_screencast_sync(connection, &params);
     ASSERT_EQ(nullptr, screencast);
-
-    mir_connection_release(connection);
 }
 
 TEST_F(MirScreencastTest, gets_valid_egl_native_window)
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     mf::ScreencastSessionId const screencast_session_id{99};
 
     InSequence seq;
@@ -217,24 +193,17 @@
     EXPECT_NE(MirEGLNativeWindowType(), egl_native_window);
 
     mir_screencast_release_sync(screencast);
-
-    mir_connection_release(connection);
 }
 
 TEST_F(MirScreencastTest, fails_on_client_when_server_request_fails)
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     EXPECT_CALL(mock_screencast(), create_session(_, _, _))
         .WillOnce(Throw(std::runtime_error("")));
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
     ASSERT_EQ(nullptr, screencast);
-
-    mir_connection_release(connection);
 }
 
 namespace
@@ -255,15 +224,10 @@
     MockSessionAuthorizer mock_authorizer;
 };
 
-class UnauthorizedMirScreencastTest : public mir_test_framework::InProcessServer
+struct UnauthorizedMirScreencastTest : mtf::BasicClientServerFixture<MockAuthorizerServerConfig>
 {
-public:
-    mir::DefaultServerConfiguration& server_config() override { return server_config_; }
-
-    MockSessionAuthorizer& mock_authorizer() { return server_config_.mock_authorizer; }
-
-    MockAuthorizerServerConfig server_config_;
-    mtf::UsingStubClientPlatform using_stub_client_platform;
+    MockSessionAuthorizer& mock_authorizer() { return server_configuration.mock_authorizer; }
+
     MirScreencastParameters default_screencast_params {
         {0, 0, 1, 1}, 1, 1, mir_pixel_format_abgr_8888};
 };

=== modified file 'tests/acceptance-tests/test_client_surface_swap_buffers.cpp'
--- tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-08-05 11:34:22 +0000
+++ tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-08-12 10:44:30 +0000
@@ -19,8 +19,7 @@
 #include "mir_toolkit/mir_client_library.h"
 
 #include "mir_test_framework/stubbed_server_configuration.h"
-#include "mir_test_framework/in_process_server.h"
-#include "mir_test_framework/using_stub_client_platform.h"
+#include "mir_test_framework/basic_client_server_fixture.h"
 #include "mir_test_doubles/null_display_buffer_compositor_factory.h"
 #include "mir_test/signal.h"
 
@@ -43,30 +42,20 @@
     }
 };
 
-class MirSurfaceSwapBuffersTest : public mir_test_framework::InProcessServer
-{
-public:
-    mir::DefaultServerConfiguration& server_config() override { return server_config_; }
-
-    StubServerConfig server_config_;
-    mtf::UsingStubClientPlatform using_stub_client_platform;
-};
-
 void swap_buffers_callback(MirSurface*, void* ctx)
 {
     auto buffers_swapped = static_cast<mt::Signal*>(ctx);
     buffers_swapped->raise();
 }
 
+using MirSurfaceSwapBuffersTest = mtf::BasicClientServerFixture<StubServerConfig>;
+
 }
 
 TEST_F(MirSurfaceSwapBuffersTest, swap_buffers_does_not_block_when_surface_is_not_composited)
 {
     using namespace testing;
 
-    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-    ASSERT_TRUE(mir_connection_is_valid(connection));
-
     MirSurfaceParameters request_params =
     {
         __PRETTY_FUNCTION__,
@@ -93,5 +82,4 @@
     }
 
     mir_surface_release_sync(surface);
-    mir_connection_release(connection);
 }

=== modified file 'tests/acceptance-tests/test_large_messages.cpp'
--- tests/acceptance-tests/test_large_messages.cpp	2014-05-19 10:30:17 +0000
+++ tests/acceptance-tests/test_large_messages.cpp	2014-08-12 10:44:30 +0000
@@ -18,6 +18,7 @@
 
 #include "mir_test_framework/in_process_server.h"
 #include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 #include "mir_test/wait_object.h"
 #include "mir_test/fake_shared.h"
 #include "mir_test_doubles/stub_display_configuration.h"
@@ -66,6 +67,8 @@
     {
         return large_messages_server_config;
     }
+
+    mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
 };
 
 struct ConnectionContext

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2014-08-06 15:15:35 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2014-08-12 10:44:30 +0000
@@ -28,6 +28,7 @@
 
 #include "mir_test_framework/in_process_server.h"
 #include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include "mir_test_doubles/mock_egl.h"
 
@@ -267,6 +268,7 @@
     NestedServer() : HostServerConfiguration(display_geometry) {}
 
     NestedMockEGL mock_egl;
+    mtf::UsingStubClientPlatform using_stub_client_platform;
 
     virtual mir::DefaultServerConfiguration& server_config()
     {

=== modified file 'tests/acceptance-tests/test_prompt_session_client_api.cpp'
--- tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-07-30 15:25:54 +0000
+++ tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-08-12 10:44:30 +0000
@@ -27,6 +27,7 @@
 #include "mir_test_doubles/stub_session_authorizer.h"
 #include "mir_test_framework/stubbed_server_configuration.h"
 #include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 #include "mir_test/popen.h"
 
 #include <gtest/gtest.h>
@@ -140,6 +141,7 @@
     std::shared_ptr<mf::Session> application_session;
 
     std::shared_ptr<ms::PromptSession> server_prompt_session;
+    mtf::UsingStubClientPlatform using_stub_client_platform;
 
     void SetUp() override
     {

=== modified file 'tests/acceptance-tests/test_protobuf.cpp'
--- tests/acceptance-tests/test_protobuf.cpp	2014-07-10 03:45:37 +0000
+++ tests/acceptance-tests/test_protobuf.cpp	2014-08-12 10:44:30 +0000
@@ -26,6 +26,7 @@
 
 #include "mir_test_framework/stubbed_server_configuration.h"
 #include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -34,6 +35,7 @@
 
 namespace mf = mir::frontend;
 namespace mfd = mir::frontend::detail;
+namespace mtf = mir_test_framework;
 
 /*************************************************************************/
 /*************************************************************************/
@@ -136,7 +138,7 @@
     }
 };
 
-struct DemoServerConfiguration : mir_test_framework::StubbedServerConfiguration
+struct DemoServerConfiguration : mtf::StubbedServerConfiguration
 {
     std::shared_ptr<mf::ConnectionCreator> the_connection_creator() override
     {
@@ -151,11 +153,12 @@
 
 };
 
-struct DemoPrivateProtobuf : mir_test_framework::InProcessServer
+struct DemoPrivateProtobuf : mtf::InProcessServer
 {
     mir::DefaultServerConfiguration& server_config() override { return my_server_config; }
 
     DemoServerConfiguration my_server_config;
+    mtf::UsingStubClientPlatform using_stub_client_platform;
 
     std::shared_ptr<DemoConnectionCreator> demo_connection_creator;
 
@@ -163,7 +166,7 @@
     {
         ::demo_mir_server = &demo_mir_server;
 
-        mir_test_framework::InProcessServer::SetUp();
+        mtf::InProcessServer::SetUp();
         demo_connection_creator = std::dynamic_pointer_cast<DemoConnectionCreator>(my_server_config.the_connection_creator());
 
         using namespace testing;

=== modified file 'tests/acceptance-tests/test_server_disconnect.cpp'
--- tests/acceptance-tests/test_server_disconnect.cpp	2014-03-26 05:48:59 +0000
+++ tests/acceptance-tests/test_server_disconnect.cpp	2014-08-12 10:44:30 +0000
@@ -130,6 +130,40 @@
     mt::CrossProcessAction configure_display;
     mt::CrossProcessAction disconnect;
 };
+
+/*
+ * This client will self-terminate on server connection break (through the default
+ * lifecycle handler).
+ */
+struct TerminatingTestingClientConfiguration : mtf::TestingClientConfiguration
+{
+    void exec() override
+    {
+        MirConnection* connection{nullptr};
+
+        connect.exec([&] {
+            connection = mir_connect_sync(mtf::test_socket_file().c_str() , __PRETTY_FUNCTION__);
+            EXPECT_TRUE(mir_connection_is_valid(connection));
+        });
+
+        create_surface_sync.wait_for_signal_ready_for();
+
+        MirSurfaceParameters const parameters =
+        {
+            __PRETTY_FUNCTION__,
+            1, 1,
+            mir_pixel_format_abgr_8888,
+            mir_buffer_usage_hardware,
+            mir_display_output_id_invalid
+        };
+        mir_connection_create_surface_sync(connection, &parameters);
+
+        mir_connection_release(connection);
+    }
+
+    mt::CrossProcessAction connect;
+    mtf::CrossProcessSync create_surface_sync;
+};
 }
 
 TEST_F(ServerDisconnect, client_detects_server_shutdown)
@@ -168,3 +202,29 @@
         client_config.disconnect();
     });
 }
+
+TEST_F(ServerDisconnect, causes_client_to_terminate_by_default)
+{
+    TestingServerConfiguration server_config;
+    launch_server_process(server_config);
+
+    TerminatingTestingClientConfiguration client_config;
+    launch_client_process(client_config);
+
+    run_in_test_process([this, &client_config]
+    {
+        client_config.connect();
+        shutdown_server_process();
+
+        /*
+         * While trying to create a surface the connection break will be detected
+         * and the client should self-terminate.
+         */
+        client_config.create_surface_sync.signal_ready();
+
+        auto const client_results = wait_for_shutdown_client_processes();
+        ASSERT_EQ(1, client_results.size());
+        EXPECT_EQ(mtf::TerminationReason::child_terminated_by_signal,
+                  client_results[0].reason);
+    });
+}

=== modified file 'tests/acceptance-tests/test_server_shutdown.cpp'
--- tests/acceptance-tests/test_server_shutdown.cpp	2014-08-07 10:27:18 +0000
+++ tests/acceptance-tests/test_server_shutdown.cpp	2014-08-12 10:44:30 +0000
@@ -329,6 +329,8 @@
 
         /* Notify the clients that we are done (we only need to set the flag once) */
         server_done.set();
+
+        wait_for_shutdown_client_processes();
     });
 
     /*

=== modified file 'tests/acceptance-tests/test_stale_frames.cpp'
--- tests/acceptance-tests/test_stale_frames.cpp	2014-07-25 10:02:18 +0000
+++ tests/acceptance-tests/test_stale_frames.cpp	2014-08-12 10:44:30 +0000
@@ -25,7 +25,6 @@
 #include "mir/graphics/buffer.h"
 #include "mir/graphics/buffer_id.h"
 
-#include "mir_test_framework/using_stub_client_platform.h"
 #include "mir_test_framework/stubbed_server_configuration.h"
 #include "mir_test_framework/basic_client_server_fixture.h"
 #include "mir_test_doubles/stub_renderer.h"
@@ -180,7 +179,6 @@
     }
 
     MirSurface* surface;
-    mtf::UsingStubClientPlatform using_stub_client_platform;
 };
 
 }

=== modified file 'tests/acceptance-tests/test_test_framework.cpp'
--- tests/acceptance-tests/test_test_framework.cpp	2014-03-06 06:05:17 +0000
+++ tests/acceptance-tests/test_test_framework.cpp	2014-08-12 10:44:30 +0000
@@ -19,6 +19,7 @@
 #include "mir_test_framework/display_server_test_fixture.h"
 #include "mir_test_framework/testing_server_configuration.h"
 #include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/using_stub_client_platform.h"
 
 #include "mir_toolkit/mir_client_library.h"
 
@@ -74,6 +75,7 @@
     virtual mir::DefaultServerConfiguration& server_config() { return server_config_; }
 
     mir_test_framework::StubbedServerConfiguration server_config_;
+    mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
 };
 }
 

=== modified file 'tests/integration-tests/client/test_client_render.cpp'
--- tests/integration-tests/client/test_client_render.cpp	2014-07-30 15:25:54 +0000
+++ tests/integration-tests/client/test_client_render.cpp	2014-08-12 10:44:30 +0000
@@ -57,6 +57,10 @@
 static mtd::DrawPatternCheckered<2,2> draw_pattern1(pattern1);
 static const char socket_file[] = "./test_client_ipc_render_socket";
 
+void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
+{
+}
+
 struct TestClient
 {
     static MirPixelFormat select_format_for_visual_id(int visual_id)
@@ -110,6 +114,9 @@
         }
 
         mir_surface_release_sync(surface);
+        // Clear the lifecycle callback in order not to get SIGTERM by the default
+        // lifecycle handler during connection teardown
+        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
         mir_connection_release(connection);
         return 0;
     }
@@ -167,6 +174,9 @@
         }
 
         mir_surface_release_sync(mir_surface);
+        // Clear the lifecycle callback in order not to get SIGTERM by the default
+        // lifecycle handler during connection teardown
+        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
         mir_connection_release(connection);
         return 0;
     }

=== modified file 'tests/integration-tests/shell/test_session_lifecycle_event.cpp'
--- tests/integration-tests/shell/test_session_lifecycle_event.cpp	2014-07-21 03:35:31 +0000
+++ tests/integration-tests/shell/test_session_lifecycle_event.cpp	2014-08-12 10:44:30 +0000
@@ -19,82 +19,71 @@
 #include "mir/scene/null_session_listener.h"
 #include "src/server/scene/application_session.h"
 
-#include "mir_test_framework/display_server_test_fixture.h"
+#include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/basic_client_server_fixture.h"
+#include "mir_test_framework/in_process_server.h"
 
 #include "mir_toolkit/mir_client_library.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
-#include <thread>
-#include <unistd.h>
-#include <fcntl.h>
-
 namespace ms = mir::scene;
-namespace msh = mir::shell;
 namespace mtf = mir_test_framework;
 
 namespace
 {
-char const* const mir_test_socket = mtf::test_socket_file().c_str();
 
 struct MockStateHandler
 {
     MOCK_METHOD1(state_changed, void (MirLifecycleState));
 };
 
+void lifecycle_callback(MirConnection* /*connection*/, MirLifecycleState state, void* context)
+{
+    auto const handler = static_cast<MockStateHandler*>(context);
+    handler->state_changed(state);
+}
+
 class StubSessionListener : public ms::NullSessionListener
 {
     void stopping(std::shared_ptr<ms::Session> const& session)
     {
-        std::shared_ptr<ms::ApplicationSession> app_session(
-            std::static_pointer_cast<ms::ApplicationSession>(session)
-            );
+        auto const app_session = std::static_pointer_cast<ms::ApplicationSession>(session);
 
         app_session->set_lifecycle_state(mir_lifecycle_state_will_suspend);
     }
 };
 
-using LifecycleEventTest = BespokeDisplayServerTestFixture;
+struct StubServerConfig : mtf::StubbedServerConfiguration
+{
+    std::shared_ptr<ms::SessionListener> the_session_listener() override
+    {
+        return std::make_shared<StubSessionListener>();
+    }
+};
+
+struct LifecycleEventTest : mtf::InProcessServer
+{
+    StubServerConfig server_configuration;
+
+    mir::DefaultServerConfiguration& server_config() override
+        { return server_configuration; }
+};
+
+}
+
 TEST_F(LifecycleEventTest, lifecycle_event_test)
 {
     using namespace ::testing;
 
-    struct ServerConfig : TestingServerConfiguration
-    {
-        std::shared_ptr<ms::SessionListener> the_session_listener() override
-        {
-            return std::make_shared<StubSessionListener>();
-        }
-    } server_config;
-
-    launch_server_process(server_config);
-
-    struct ClientConfig : TestingClientConfiguration
-    {
-        static void lifecycle_callback(MirConnection* /*connection*/, MirLifecycleState state, void* context)
-        {
-            auto config = static_cast<ClientConfig*>(context);
-            config->handler->state_changed(state);
-        }
-
-        void exec()
-        {
-            handler = std::make_shared<MockStateHandler>();
-
-            MirConnection* connection = mir_connect_sync(mir_test_socket, "testapp");
-            mir_connection_set_lifecycle_event_callback(connection, lifecycle_callback, this);
-
-            EXPECT_CALL(*handler, state_changed(Eq(mir_lifecycle_state_will_suspend))).Times(1);
-            EXPECT_CALL(*handler, state_changed(Eq(mir_lifecycle_connection_lost))).Times(AtMost(1));
-            mir_connection_release(connection);
-
-            handler.reset();
-        }
-
-        std::shared_ptr<MockStateHandler> handler;
-    } client_config;
-
-    launch_client_process(client_config);
-}
+    auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    auto const handler = std::make_shared<MockStateHandler>();
+    mir_connection_set_lifecycle_event_callback(connection, lifecycle_callback, handler.get());
+
+    EXPECT_CALL(*handler, state_changed(Eq(mir_lifecycle_state_will_suspend))).Times(1);
+    EXPECT_CALL(*handler, state_changed(Eq(mir_lifecycle_connection_lost))).Times(AtMost(1));
+
+    mir_connection_release(connection);
 }

=== modified file 'tests/mir_test_framework/display_server_test_fixture.cpp'
--- tests/mir_test_framework/display_server_test_fixture.cpp	2014-04-10 22:23:27 +0000
+++ tests/mir_test_framework/display_server_test_fixture.cpp	2014-08-12 10:44:30 +0000
@@ -87,6 +87,11 @@
     return process_manager.wait_for_shutdown_server_process();
 }
 
+std::vector<mtf::Result> BespokeDisplayServerTestFixture::wait_for_shutdown_client_processes()
+{
+    return process_manager.wait_for_shutdown_client_processes();
+}
+
 void BespokeDisplayServerTestFixture::terminate_client_processes()
 {
     process_manager.terminate_client_processes();

=== modified file 'tests/mir_test_framework/testing_process_manager.cpp'
--- tests/mir_test_framework/testing_process_manager.cpp	2014-04-10 22:23:27 +0000
+++ tests/mir_test_framework/testing_process_manager.cpp	2014-08-12 10:44:30 +0000
@@ -203,6 +203,18 @@
     return result;
 }
 
+std::vector<mtf::Result> mtf::TestingProcessManager::wait_for_shutdown_client_processes()
+{
+    std::vector<Result> results;
+
+    for (auto const& client : clients)
+        results.push_back(client->wait_for_termination());
+
+    clients.clear();
+
+    return results;
+}
+
 void mtf::TestingProcessManager::terminate_client_processes()
 {
     if (is_test_process)

=== modified file 'tests/mir_test_framework/using_stub_client_platform.cpp'
--- tests/mir_test_framework/using_stub_client_platform.cpp	2014-08-05 10:18:13 +0000
+++ tests/mir_test_framework/using_stub_client_platform.cpp	2014-08-12 10:44:30 +0000
@@ -18,6 +18,7 @@
 
 #include "mir_test_framework/using_stub_client_platform.h"
 #include "mir_test_framework/stub_client_connection_configuration.h"
+#include "mir_toolkit/mir_client_library.h"
 #include "src/client/mir_connection_api.h"
 
 namespace mtf = mir_test_framework;
@@ -26,6 +27,10 @@
 namespace
 {
 
+void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
+{
+}
+
 class StubMirConnectionAPI : public mcl::MirConnectionAPI
 {
 public:
@@ -45,6 +50,9 @@
 
     void release(MirConnection* connection) override
     {
+        // Clear the lifecycle callback in order not to get SIGTERM by the default
+        // lifecycle handler during connection teardown
+        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
         return prev_api->release(connection);
     }
 

=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2014-07-30 15:25:54 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2014-08-12 10:44:30 +0000
@@ -392,6 +392,10 @@
 {
 }
 
+void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
+{
+}
+
 MATCHER_P(BufferPackageMatches, package, "")
 {
     // Can't simply use memcmp() on the whole struct because age is not sent over the wire
@@ -415,6 +419,13 @@
         connect_to_test_server();
     }
 
+    ~MirClientSurfaceTest()
+    {
+        // Clear the lifecycle callback in order not to get SIGTERM by the default
+        // lifecycle handler during connection teardown
+        connection->register_lifecycle_event_callback(null_lifecycle_callback, nullptr);
+    }
+
     void start_test_server()
     {
         // In case an earlier test left a stray file