Merge lp:~alan-griffiths/mir/mir_lifecycle_connection_lost into lp:mir

[Alan Griffiths]

client & tests: Provide a lifecycle callback when server connection is lost.

client & tests: Provide a lifecycle callback when server connection is lost.

The client can register a handler using mir_connection_set_lifecycle_event_callback() this will then receive a new event "mir_lifecycle_connection_lost" when a server connection dies. After this the connection (and associated surfaces and buffers) is useless and should be closed down.

Typically, a client app might want to save state & exit and/or try to build a new connection.

If the client hasn't registered a callback handler, then use we a default handler that raises SIGTERM - as that leads to sensible behavior in the examples.

[Alan Griffiths (alan-griffiths)]

I've found an intermittent failure in this test - will fix once I've diagnosed it.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1106
http://jenkins.qa.ubuntu.com/job/mir-ci/1579/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2227
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/823
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/823/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/80
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/80/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1110
http://jenkins.qa.ubuntu.com/job/mir-ci/1582/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2235
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2120
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/826
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/826/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/83
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/83/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Kevin DuBois (kdub)]

looks good to me too

review: Approve

[Alan Griffiths (alan-griffiths)]

Resubmitted with correct target

[Kevin DuBois (kdub)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1110
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/92/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2238
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2123
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/91
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/91/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h	2013-09-12 21:36:55 +0000
+++ include/shared/mir_toolkit/common.h	2013-10-04 16:55:04 +0000
@@ -76,7 +76,8 @@
 typedef enum MirLifecycleState
 {
     mir_lifecycle_state_will_suspend,
-    mir_lifecycle_state_resumed
+    mir_lifecycle_state_resumed,
+    mir_lifecycle_connection_lost
 } MirLifecycleState;
 
 typedef enum MirPowerMode

=== modified file 'src/client/lifecycle_control.cpp'
--- src/client/lifecycle_control.cpp	2013-08-23 12:23:47 +0000
+++ src/client/lifecycle_control.cpp	2013-10-04 16:55:04 +0000
@@ -18,8 +18,6 @@
 
 #include "lifecycle_control.h"
 
-#include <memory>
-
 namespace mcl = mir::client;
 
 mcl::LifecycleControl::LifecycleControl()

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-10-03 03:44:08 +0000
+++ src/client/mir_client_library.cpp	2013-10-04 16:55:04 +0000
@@ -345,9 +345,14 @@
 }
 
 MirWaitHandle* mir_surface_swap_buffers(MirSurface *surface, mir_surface_callback callback, void * context)
+try
 {
     return surface->next_buffer(callback, context);
 }
+catch (std::exception const&)
+{
+    return 0;
+}
 
 void mir_surface_swap_buffers_sync(MirSurface *surface)
 {

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-10-03 03:44:08 +0000
+++ src/client/mir_connection.cpp	2013-10-04 16:55:04 +0000
@@ -31,6 +31,7 @@
 #include <algorithm>
 #include <cstddef>
 #include <unistd.h>
+#include <signal.h>
 
 namespace mcl = mir::client;
 namespace mircv = mir::input::receiver;
@@ -201,6 +202,18 @@
     return new_wait_handle;
 }
 
+namespace
+{
+void default_lifecycle_event_handler(MirLifecycleState transition)
+{
+    if (transition == mir_lifecycle_connection_lost)
+    {
+        raise(SIGTERM);
+    }
+}
+}
+
+
 void MirConnection::connected(mir_connected_callback callback, void * context)
 {
     bool safe_to_callback = true;
@@ -225,6 +238,7 @@
         platform = client_platform_factory->create_client_platform(this);
         native_display = platform->create_egl_native_display();
         display_configuration->set_configuration(connect_result.display_configuration());
+        lifecycle_control->set_lifecycle_event_handler(default_lifecycle_event_handler);
     }
 
     if (safe_to_callback) callback(this, context);

=== modified file 'src/client/rpc/mir_socket_rpc_channel.cpp'
--- src/client/rpc/mir_socket_rpc_channel.cpp	2013-10-03 03:44:08 +0000
+++ src/client/rpc/mir_socket_rpc_channel.cpp	2013-10-04 16:55:04 +0000
@@ -30,19 +30,12 @@
 
 #include <boost/exception/errinfo_errno.hpp>
 #include <boost/throw_exception.hpp>
-
 #include <boost/bind.hpp>
-#include <boost/throw_exception.hpp>
 
-#include <cstring>
-#include <sstream>
 #include <stdexcept>
 
-#include <pthread.h>
 #include <signal.h>
 
-#include <stdio.h>
-
 namespace mcl = mir::client;
 namespace mclr = mir::client::rpc;
 
@@ -58,7 +51,8 @@
     socket(io_service),
     surface_map(surface_map),
     display_configuration(disp_config),
-    lifecycle_control(lifecycle_control)
+    lifecycle_control(lifecycle_control),
+    disconnected(false)
 {
     socket.connect(endpoint);
     init();
@@ -76,7 +70,8 @@
     socket(io_service),
     surface_map(surface_map),
     display_configuration(disp_config),
-    lifecycle_control(lifecycle_control)
+    lifecycle_control(lifecycle_control),
+    disconnected(false)
 {
     socket.assign(boost::asio::local::stream_protocol(), native_socket);
     init();
@@ -84,10 +79,12 @@
 
 void mclr::MirSocketRpcChannel::notify_disconnected()
 {
-    // TODO enable configuring the kill mechanism
-    io_service.stop();
-    raise (SIGTERM);
-    pending_calls.force_completion();
+    if (!disconnected.exchange(true))
+    {
+        io_service.stop();
+        lifecycle_control->call_lifecycle_event_handler(mir_lifecycle_connection_lost);
+        pending_calls.force_completion();
+    }
 }
 
 void mclr::MirSocketRpcChannel::init()
@@ -288,6 +285,7 @@
     if (error)
     {
         rpc_report->invocation_failed(invocation, error);
+        notify_disconnected();
         BOOST_THROW_EXCEPTION(std::runtime_error("Failed to send message to server: " + error.message()));
     }
     else

=== modified file 'src/client/rpc/mir_socket_rpc_channel.h'
--- src/client/rpc/mir_socket_rpc_channel.h	2013-10-03 03:44:08 +0000
+++ src/client/rpc/mir_socket_rpc_channel.h	2013-10-04 16:55:04 +0000
@@ -98,6 +98,7 @@
     std::shared_ptr<SurfaceMap> surface_map;
     std::shared_ptr<DisplayConfiguration> display_configuration;
     std::shared_ptr<LifecycleControl> lifecycle_control;
+    std::atomic<bool> disconnected;
 };
 
 }

=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2013-09-05 17:44:44 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2013-10-04 16:55:04 +0000
@@ -16,6 +16,7 @@
   test_nested_mir.cpp
   test_display_configuration.cpp
   test_surfaces_with_output_id.cpp
+  test_server_disconnect.cpp
 )
 
 list(APPEND SOURCES

=== added file 'tests/acceptance-tests/test_server_disconnect.cpp'
--- tests/acceptance-tests/test_server_disconnect.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_server_disconnect.cpp	2013-10-04 16:55:04 +0000
@@ -0,0 +1,93 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alan Griffiths <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+
+#include "mir_test_framework/display_server_test_fixture.h"
+#include "mir_test_framework/cross_process_sync.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <atomic>
+
+namespace mtf = mir_test_framework;
+
+using ServerDisconnect = mtf::BespokeDisplayServerTestFixture;
+
+namespace
+{
+struct MockEventHandler
+{
+    MOCK_METHOD1(handle, void(MirLifecycleState transition));
+
+    static void handle(MirConnection*, MirLifecycleState transition, void* event_handler)
+    {
+        static_cast<MockEventHandler*>(event_handler)->handle(transition);
+    }
+};
+
+struct MyTestingClientConfiguration : mtf::TestingClientConfiguration
+{
+    void exec() override
+    {
+        static MirSurfaceParameters const params =
+            { __PRETTY_FUNCTION__, 33, 45, mir_pixel_format_abgr_8888,
+              mir_buffer_usage_hardware, mir_display_output_id_invalid };
+
+        MockEventHandler mock_event_handler;
+
+        auto connection = mir_connect_sync(mtf::test_socket_file().c_str() , __PRETTY_FUNCTION__);
+        mir_connection_set_lifecycle_event_callback(connection, &MockEventHandler::handle, &mock_event_handler);
+        auto surface = mir_connection_create_surface_sync(connection, &params);
+
+        std::atomic<bool> signalled(false);
+
+        EXPECT_CALL(mock_event_handler, handle(mir_lifecycle_connection_lost)).Times(1).
+            WillOnce(testing::InvokeWithoutArgs([&] { signalled.store(true); }));
+
+        sync.signal_ready();
+
+        using clock = std::chrono::high_resolution_clock;
+
+        auto time_limit = clock::now() + std::chrono::seconds(2);
+
+        while (!signalled.load() && clock::now() < time_limit)
+        {
+            mir_surface_swap_buffers_sync(surface);
+        }
+    }
+
+    mtf::CrossProcessSync sync;
+};
+}
+
+TEST_F(ServerDisconnect, client_detects_server_shutdown)
+{
+    TestingServerConfiguration server_config;
+    launch_server_process(server_config);
+
+    MyTestingClientConfiguration client_config;
+    launch_client_process(client_config);
+
+    run_in_test_process([this, &client_config]
+    {
+        client_config.sync.wait_for_signal_ready_for();
+        shutdown_server_process();
+    });
+}

=== modified file 'tests/unit-tests/frontend/test_published_socket_connector.cpp'
--- tests/unit-tests/frontend/test_published_socket_connector.cpp	2013-09-27 16:06:10 +0000
+++ tests/unit-tests/frontend/test_published_socket_connector.cpp	2013-10-04 16:55:04 +0000
@@ -221,12 +221,10 @@
     Mock::VerifyAndClearExpectations(client.get());
 
     EXPECT_CALL(*client->rpc_report, invocation_failed(InvocationMethodEq("disconnect"),_));
-    EXPECT_CALL(*client, disconnect_done()).Times(0);
+    EXPECT_CALL(*client, disconnect_done()).Times(1);
 
     EXPECT_THROW({
-    // We don't know if this will be called, so it can't auto destruct
-    std::unique_ptr<google::protobuf::Closure> new_callback(google::protobuf::NewPermanentCallback(client.get(), &mt::TestProtobufClient::disconnect_done));
-    client->display_server.disconnect(0, &client->ignored, &client->ignored, new_callback.get());
+    client->display_server.disconnect(0, &client->ignored, &client->ignored, google::protobuf::NewCallback(client.get(), &mt::TestProtobufClient::disconnect_done));
     client->wait_for_disconnect_done();
     }, std::runtime_error);
 }