Mir

Merge lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver into lp:mir

use-lifecycleevent-in-mirserver
Merge into development-branch

Proposed by Andreas Pokorny on 2016-04-20

Status:	Rejected
Rejected by:	Andreas Pokorny on 2016-06-13
Proposed branch:	lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver
Merge into:	lp:mir
Diff against target:	580 lines (+289/-32) 11 files modified src/include/server/mir/default_server_status_listener.h (+12/-12) src/server/CMakeLists.txt (+1/-0) src/server/default_server_configuration.cpp (+2/-2) src/server/default_server_status_listener.cpp (+50/-0) src/server/graphics/nested/display.cpp (+70/-7) src/server/graphics/nested/display.h (+15/-0) src/server/graphics/nested/host_connection.h (+1/-0) src/server/graphics/nested/mir_client_host_connection.cpp (+27/-8) src/server/graphics/nested/mir_client_host_connection.h (+3/-0) tests/include/mir/test/doubles/stub_host_connection.h (+4/-0) tests/unit-tests/graphics/nested/test_nested_display.cpp (+104/-3)
To merge this branch:	bzr merge lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Approve on 2016-06-10
Alan Griffiths		2016-04-20	Approve on 2016-06-09
Chris Halse Rogers			Needs Fixing on 2016-04-29
Alberto Aguirre (community)			Needs Information on 2016-04-27
Brandon Schaefer (community)			Approve on 2016-04-26
Cemil Azizoglu (community)			Approve on 2016-04-26
Review via email: mp+292375@code.launchpad.net

Commit message

Use the builtin lifecycle event as an indication of lifecycle changes of the server

With this change all sessions will receive lifecycle events when the server pauses or resumes. The nested display will use this event to feed the registered pause/resume handlers. Thus a nested server will pause when the hosting server pauses due to a vt switch.

This is a preparation step to use resume events and focus changes as a trigger for informing clients about missed key state changes.

Description of the change

This is a first step towards the sticky-alt-key problem that may happen after vt switches.
With this change the nested session is aware that the system compositor is paused, and gets paused to.

I also spiked using focus events as an indication, but that leads to several problematic scenarios during display configuration changes.

The proposed server status handling in DefaultServerStatusListener is identical to what unity-system-compositor already does. Note that also qtmir overrides this class since it has its own life cycle management that depends on the current shell mode.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-20:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/871/rebuild

review: Approve (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-25:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/894/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-25:

-ctest -S "$TEST_DIR/steer.cmake" --output-on-failure -j8 -V $@
+ctest -S "$TEST_DIR/steer.cmake" --output-on-failure -j16 -V $@

Doesn't belong in this MP.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-25:

"With this change the active session will receive lifecycle events when the server pauses or resumes. The nested display will use this event to feed the registered pause/resume handlers. Thus a nested server will pause when the hosting server pauses due to a vt switch."

Don't all sessions need the lifecycle event?

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-04-26:

> "With this change the active session will receive lifecycle events when the
> server pauses or resumes. The nested display will use this event to feed the
> registered pause/resume handlers. Thus a nested server will pause when the
> hosting server pauses due to a vt switch."
>
> Don't all sessions need the lifecycle event?

Good point..

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-26:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/903/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-26:

Nit:

+ enum DisplayState
+ {
+ running,
+ paused,
+ };

Our guidelines say to prefer scoped enums (i.e. "enum class")

review: Approve

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-04-26:

LGTM

review: Approve

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2016-04-26:

lgtm

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-26:

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-04-27:

20:55:57 /��BUILDDIR��/mir-0.22.0+vivid977bzr3469/src/server/graphics/nested/display.cpp:217:13: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
20:55:57 write(lifecycle_trigger, &dummy, sizeof dummy);
20:55:57 ^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-04-27:

174 + uint64_t dummy = 1;
175 + write(lifecycle_trigger, &dummy, sizeof dummy);

This can be slightly simplified as eventfd_write(lifecycle_trigger, 1); That would also fix the build issue with ignoring the return value of write().

But you should probably be checking for error and throwing if so; otherwise we'll get unexpected strange behaviour later on in the extremely unlikely event that eventfd_write fails.

Needs-fixing based on that.

Now that I think of it, I'm somewhat concerned about us overloading the meaning of mir_lifecycle_event_will_suspend and mir_lifecycle_event_resumed. For applications these mean “serialise your state to disc, because I'm about to kill you” and “restore your state from disc, because I've started you up again after killing you” respectively.

I know that using focus events for this ran into the problem of spurious focus changes during display configuration changes; would fixing those spurious focus changes be easy enough, and allow us to just use update-keystate-on-focus-change everywhere?

review: Needs Information

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-04-27:

> Now that I think of it, I'm somewhat concerned about us overloading the
> meaning of mir_lifecycle_event_will_suspend and mir_lifecycle_event_resumed.
> For applications these mean “serialise your state to disc, because I'm about
> to kill you” and “restore your state from disc, because I've started you up
> again after killing you” respectively.
>
> I know that using focus events for this ran into the problem of spurious focus
> changes during display configuration changes; would fixing those spurious
> focus changes be easy enough, and allow us to just use update-keystate-on-
> focus-change everywhere?

I will look into the second part. On the lifecycle event I thought unity8 also does that, and might not kill the application between the two events, but I might be wrong here. I had a similar concern of overloading semantics when using focus lost as an indication that the server should pause.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-04-27:

Yeah, it'd be really nice if we actually had a separate
system-compositor protocol. unity-system-compositor isn't a normal Mir
server, and Unity8 isn't a normal Mir client, and it's been a continual
source of friction.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-04-27:

Actually, back up. Why are we pausing the nested server on focus-lost?

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2016-04-27:

Why overload the lifecycle semantics? Would it be too redudant to add: "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?

review: Needs Information

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-04-28:

> Why overload the lifecycle semantics? Would it be too redudant to add:
> "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?

What do these mean to non-system-compositor clients?

lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver updated on 2016-04-28

3470. By Andreas Pokorny on 2016-04-27: merge current lp:mir
3471. By Andreas Pokorny on 2016-04-28: throw on write

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-28:

> Why overload the lifecycle semantics? Would it be too redudant to add:
> "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?

They mean the server is pausing/resuming the client.

I don't think they should be used here.

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-04-28:

> > Why overload the lifecycle semantics? Would it be too redudant to add:
> > "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?
>
> What do these mean to non-system-compositor clients?

The same actually.. the change makes the server state transitive to the nested server. And the default nested server would decide that when paused clients are paused too. When resumed all clients are resumed to their previous state too.

I think that part of the behavior would be a shell decision. I.e. a nested servers could still also decide to send lifecycle stop to clients when paused.. and use the resume flank to restart the clients.

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-04-28:

> > Why overload the lifecycle semantics? Would it be too redudant to add:
> > "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?
>
> They mean the server is pausing/resuming the client.
>
> I don't think they should be used here.

Whats the need fixing?

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-04-28:

> Actually, back up. Why are we pausing the nested server on focus-lost?

Because we can, but we do not have to. This has the advantage that it would stop repeat handling and application not-responding detection in a system state in which we would be happy if applications are not responding.

We could install a specific handler for either surface attribute focus, or life cycle events. And then not pause the server.

I just believe it makes sense to use that information and pause processing.
Not only for power saving reasons but also to make the nested server less different from a system compositor.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-28:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/916/rebuild

review: Needs Fixing (continuous-integration)

lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver updated on 2016-04-29

3472. By Andreas Pokorny on 2016-04-29: enum class instead of enum

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-04-29:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/920/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-04-29:

> > > Why overload the lifecycle semantics? Would it be too redudant to add:
> > > "mir_lifecycle_event_will_pause, mir_lifecycle_event_will_resume"?
> >
> > What do these mean to non-system-compositor clients?
>
> The same actually.. the change makes the server state transitive to the nested
> server. And the default nested server would decide that when paused clients
> are paused too. When resumed all clients are resumed to their previous state
> too.
>
> I think that part of the behavior would be a shell decision. I.e. a nested
> servers could still also decide to send lifecycle stop to clients when
> paused.. and use the resume flank to restart the clients.
But we already have pause/resume events for clients, but with different semantics. Do we now have two different levels of “paused” - paused-and-SIGHUP/SIGKILL-is-moments-away and paused-and-nothing-much? And two different levels of “resumed” - resume-and-reload-from-disc-because-we've-SIGKILLed-you-in-the-past, and resume-nothing-much?

It seems like forwarding on the paused lifecycle events is shell-specific, and should not be in the DefaultServerListenerThingy().

I'm not too adverse to USC sending _will_suspend and _resumed events and nested acting on them, but I don't think the former bit should be here?

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-29:

+ if (lifecycle_callback)
+ lifecycle_callback(state);

It would be better to initialize lifecycle_callback with a null functor than rely on checking at the call site.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-04-29:

*Needs discussion*

OK, I've thought about it and read the spec.

I think the confusion comes from a mismatch between MirLifecycleState and the design.

"Event: application_about_to_suspend

"This event is triggered when the application is about to be suspended. Application authors can rely on this event to account for the situation that the UI will not be visible to the user anymore and that they will be granted at most minor CPU timeslices and no GPU timeslices. The event is accompanied by an archive that application authors can rely on to store application-specific state. In addition, an application instance is granted a limited amount of time for carrying out any preparations to go to suspend. After this time has elapsed, the system executes the transition." (Draft: Application Model)

It does makes sense to automatically propagate this to clients. But is that what mir_lifecycle_event_will_pause means? I think it has been conflated with:

"Event: application_about_to_stop

"This event is triggered when the application is about to be terminated, i.e., the process containing the application instance will be terminated. The event is accompanied by an archive that application authors can rely on to store application-specific state. In addition, an application instance is granted a limited amount of time for carrying out any preparations to go to suspend. After this time has elapsed, the system executes the transition."

review: Needs Information

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-06-09:

> *Needs discussion*
>
> OK, I've thought about it and read the spec.
>
> I think the confusion comes from a mismatch between MirLifecycleState and the
> design.

After hearing more, things are not clear and this MP doesn't add any new confusion.

review: Approve

lp:~andreas-pokorny/mir/use-lifecycleevent-in-mirserver updated on 2016-06-10

3473. By Andreas Pokorny on 2016-06-10: initialize lifecycle event callback
3474. By Andreas Pokorny on 2016-06-10: merge lp:mir and resolve if(functor) functor() complaint..

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-06-10:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1156/rebuild

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-06-13:

After a short discussion we realized that mir needs to have a different behavior on vt switch:

https://trello.com/c/cgF8ho09/407-better-pause-state-handling-in-host-server

and in general we should not have to use mir lifecycle for that..

so rejecting this MP & branch

Unmerged revisions

3474. By Andreas Pokorny on 2016-06-10

merge lp:mir and resolve if(functor) functor() complaint..

3473. By Andreas Pokorny on 2016-06-10

initialize lifecycle event callback

3472. By Andreas Pokorny on 2016-04-29

enum class instead of enum

3471. By Andreas Pokorny on 2016-04-28

throw on write

3470. By Andreas Pokorny on 2016-04-27

merge current lp:mir

3469. By Andreas Pokorny on 2016-04-26

Emit life cycle event to all sessions and revert ctest changes

3468. By Andreas Pokorny on 2016-04-25

update lp:mir

3467. By Andreas Pokorny on 2016-04-20

Use the builtin lifecycle event as an indication of lifecycle changes of the server

With this change the active session will receive lifecycle events when the server pauses or resumes. The nested display will use this event to feed the registered pause/resume handlers. Thus a nested server will pause when the hosting server pauses due to a vt switch.

This is a preparation step to use resume events and focus events as a trigger for informing clients about missed key state changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Andreas Pokorny

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'src/include/server/mir/default_server_status_listener.h'
 --- src/include/server/mir/default_server_status_listener.h	2015-02-22 07:46:25 +0000
 +++ src/include/server/mir/default_server_status_listener.h	2016-06-10 18:31:23 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2013 Canonical Ltd.
++ * Copyright © 2013,2016 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,
@@ -20,23 +20,23 @@
  #define MIR_DEFAULT_SERVER_STATUS_LISTENER_H_
  #include "mir/server_status_listener.h"
++#include "mir/scene/session.h"
  namespace mir
+ {
++namespace scene
++{
++class SessionContainer;
++}
  class DefaultServerStatusListener : public virtual ServerStatusListener
+ {
  public:
--    virtual void paused()
--    {
--    }
--
--    virtual void resumed()
--    {
--    }
--
--    virtual void started()
--    {
--    }
++    DefaultServerStatusListener(std::shared_ptr<scene::SessionContainer> const& session_container);
++    void paused() override;
++    void resumed() override;
++    void started() override;
++private:
++    std::shared_ptr<scene::SessionContainer> const session_container;
  };
+ }
 === modified file 'src/server/CMakeLists.txt'
 --- src/server/CMakeLists.txt	2016-06-02 05:33:50 +0000
 +++ src/server/CMakeLists.txt	2016-06-10 18:31:23 +0000
@@ -52,6 +52,7 @@
    terminate_with_current_exception.cpp
    display_server.cpp
    default_server_configuration.cpp
++  default_server_status_listener.cpp
    glib_main_loop.cpp
    glib_main_loop_sources.cpp
    default_emergency_cleanup.cpp
 === modified file 'src/server/default_server_configuration.cpp'
 --- src/server/default_server_configuration.cpp	2016-06-02 05:33:50 +0000
 +++ src/server/default_server_configuration.cpp	2016-06-10 18:31:23 +0000
@@ -166,9 +166,9 @@
  std::shared_ptr<mir::ServerStatusListener> mir::DefaultServerConfiguration::the_server_status_listener()
+ {
      return server_status_listener(
--        []()
++        [this]()
+         {
--            return std::make_shared<mir::DefaultServerStatusListener>();
++            return std::make_shared<mir::DefaultServerStatusListener>(the_session_container());
          });
+ }
 === added file 'src/server/default_server_status_listener.cpp'
 --- src/server/default_server_status_listener.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/default_server_status_listener.cpp	2016-06-10 18:31:23 +0000
@@ -0,0 +1,50 @@
++/*
++ * Copyright © 2016 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "mir/default_server_status_listener.h"
++#include "scene/session_container.h"
++
++#include "mir/scene/session.h"
++
++mir::DefaultServerStatusListener::DefaultServerStatusListener(
++    std::shared_ptr<scene::SessionContainer> const& session_container)
++    : session_container{session_container}
++{
++}
++
++void mir::DefaultServerStatusListener::paused()
++{
++    session_container->for_each(
++        [](std::shared_ptr<scene::Session> const& session)
++        {
++            session->set_lifecycle_state(mir_lifecycle_state_will_suspend);
++        });
++}
++
++void mir::DefaultServerStatusListener::resumed()
++{
++    session_container->for_each(
++        [](std::shared_ptr<scene::Session> const& session)
++        {
++            session->set_lifecycle_state(mir_lifecycle_state_resumed);
++        });
++}
++
++void mir::DefaultServerStatusListener::started()
++{
++}
 === modified file 'src/server/graphics/nested/display.cpp'
 --- src/server/graphics/nested/display.cpp	2016-05-20 13:08:56 +0000
 +++ src/server/graphics/nested/display.cpp	2016-06-10 18:31:23 +0000
@@ -26,6 +26,7 @@
  #include "mir/graphics/gl_context.h"
  #include "mir/graphics/surfaceless_egl_context.h"
  #include "mir/graphics/display_configuration_policy.h"
++#include "mir/graphics/event_handler_register.h"
  #include "mir/graphics/overlapping_output_grouping.h"
  #include "mir/graphics/gl_config.h"
  #include "mir/graphics/egl_error.h"
@@ -33,6 +34,7 @@
  #include "mir_toolkit/mir_connection.h"
  #include "mir/raii.h"
++#include <sys/eventfd.h>
  #include <boost/throw_exception.hpp>
  #include <stdexcept>
  #include <sstream>
@@ -186,8 +188,14 @@
      display_report{display_report},
      egl_display{connection->egl_native_display(), gl_config},
      outputs{},
--    current_configuration(std::make_unique<NestedDisplayConfiguration>(connection->create_display_config()))
++    current_configuration(std::make_unique<NestedDisplayConfiguration>(connection->create_display_config())),
++    lifecycle_trigger{eventfd(0, EFD_CLOEXEC|EFD_NONBLOCK|EFD_SEMAPHORE)}
+ {
++    if (lifecycle_trigger < 0)
++        BOOST_THROW_EXCEPTION((std::system_error{errno,
++                                                 std::system_category(),
++                                                 "Failed to create event fd for nested display"}));
++
      decltype(current_configuration) conf{dynamic_cast<NestedDisplayConfiguration*>(current_configuration->clone().release())};
      initial_conf_policy->apply_to(*conf);
@@ -198,11 +206,27 @@
          swap(current_configuration, conf);
+     }
++    connection->set_lifecycle_event_callback(
++        [this](MirLifecycleState state)
++        {
++            std::lock_guard<std::mutex> lock_display_state(pause_resume_status_mutex);
++            lifecycle_state = state;
++            uint64_t dummy = 1;
++            if (write(lifecycle_trigger, &dummy, sizeof dummy) != sizeof dummy)
++                BOOST_THROW_EXCEPTION((std::system_error{errno,
++                                                         std::system_category(),
++                                                         "Failed to notify lifecycle event fd notification"}));
++
++        });
++
      create_surfaces(*current_configuration);
+ }
  mgn::Display::~Display() noexcept
+ {
++    connection->set_lifecycle_event_callback([](MirLifecycleState){});
++    if (registered)
++        registered->unregister_fd_handler(this);
      eglBindAPI(MIR_SERVER_EGL_OPENGL_API);
      if (eglGetCurrentContext() == egl_display.egl_context())
          eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
@@ -343,21 +367,33 @@
+ }
  void mgn::Display::register_pause_resume_handlers(
--        EventHandlerRegister& /*handlers*/,
--        DisplayPauseHandler const& /*pause_handler*/,
--        DisplayResumeHandler const& /*resume_handler*/)
++        EventHandlerRegister& handlers,
++        DisplayPauseHandler const& pause_handler,
++        DisplayResumeHandler const& resume_handler)
+ {
--    // No need to do anything
++    registered = &handlers;
++    handlers.register_fd_handler(
++        {int(lifecycle_trigger)},
++        this,
++        [this](int)
++        {
++            update_pause_state();
++        });
++    std::lock_guard<std::mutex> lock_display_state(pause_resume_status_mutex);
++    this->pause_handler = pause_handler;
++    this->resume_handler = resume_handler;
+ }
  void mgn::Display::pause()
+ {
--    // No need to do anything
++    std::lock_guard<std::mutex> lock_display_state(pause_resume_status_mutex);
++    display_state = DisplayState::paused;
+ }
  void mgn::Display::resume()
+ {
--    // No need to do anything
++    std::lock_guard<std::mutex> lock_display_state(pause_resume_status_mutex);
++    display_state = DisplayState::running;
+ }
  auto mgn::Display::create_hardware_cursor(std::shared_ptr<mg::CursorImage> const& /*initial_image*/) -> std::shared_ptr<mg::Cursor>
@@ -375,3 +411,30 @@
+ {
      return nullptr;
+ }
++
++void mgn::Display::update_pause_state()
++{
++    uint64_t dummy;
++    if (read(lifecycle_trigger, &dummy, sizeof dummy) != sizeof dummy)
++    {
++        if (errno == EAGAIN)
++        {
++            return;
++        }
++        BOOST_THROW_EXCEPTION((std::system_error{errno,
++                                                 std::system_category(),
++                                                 "Failed to consume nested fd notification"}));
++    }
++
++    std::unique_lock<std::mutex> lock_display_state(pause_resume_status_mutex);
++    if (display_state != DisplayState::running && lifecycle_state == mir_lifecycle_state_resumed && resume_handler)
++    {
++        lock_display_state.unlock();
++        resume_handler();
++    }
++    else if (display_state != DisplayState::paused && lifecycle_state == mir_lifecycle_state_will_suspend && pause_handler)
++    {
++        lock_display_state.unlock();
++        pause_handler();
++    }
++}
 === modified file 'src/server/graphics/nested/display.h'
 --- src/server/graphics/nested/display.h	2016-05-20 13:08:56 +0000
 +++ src/server/graphics/nested/display.h	2016-06-10 18:31:23 +0000
@@ -23,6 +23,7 @@
  #include "mir/graphics/display_buffer.h"
  #include "mir/graphics/display_configuration.h"
  #include "mir/graphics/egl_resources.h"
++#include "mir/fd.h"
  #include "mir_toolkit/client_types.h"
@@ -154,8 +155,22 @@
      std::mutex mutable configuration_mutex;
      std::unique_ptr<NestedDisplayConfiguration> current_configuration;
++    std::mutex pause_resume_status_mutex;
++    mir::Fd lifecycle_trigger;
++    EventHandlerRegister* registered{nullptr};
++    enum class DisplayState
++    {
++        running,
++        paused,
++    };
++    DisplayState display_state{DisplayState::running};
++    MirLifecycleState lifecycle_state{mir_lifecycle_connection_lost};
++    DisplayPauseHandler pause_handler;
++    DisplayResumeHandler resume_handler;
++
      void create_surfaces(mir::graphics::DisplayConfiguration const& configuration);
      void complete_display_initialization(MirPixelFormat format);
++    void update_pause_state();
  };
+ }
 === modified file 'src/server/graphics/nested/host_connection.h'
 --- src/server/graphics/nested/host_connection.h	2016-06-07 16:20:53 +0000
 +++ src/server/graphics/nested/host_connection.h	2016-06-10 18:31:23 +0000
@@ -56,6 +56,7 @@
      virtual void set_cursor_image(CursorImage const& image) = 0;
      virtual void hide_cursor() = 0;
      virtual auto graphics_platform_library() -> std::string = 0;
++    virtual void set_lifecycle_event_callback(std::function<void(MirLifecycleState)> const& cb) = 0;
      virtual UniqueInputConfig create_input_device_config() = 0;
      virtual void set_input_device_change_callback(std::function<void(UniqueInputConfig)> const& cb) = 0;
 === modified file 'src/server/graphics/nested/mir_client_host_connection.cpp'
 --- src/server/graphics/nested/mir_client_host_connection.cpp	2016-06-09 13:15:34 +0000
 +++ src/server/graphics/nested/mir_client_host_connection.cpp	2016-06-10 18:31:23 +0000
@@ -29,6 +29,7 @@
  #include "mir/input/input_device_observer.h"
  #include "mir/frontend/event_sink.h"
  #include "mir/server_action_queue.h"
++#include "mir/report_exception.h"
  #include <boost/throw_exception.hpp>
  #include <boost/exception/errinfo_errno.hpp>
@@ -62,12 +63,6 @@
      *reply_ptr = reply;
+ }
--static void nested_lifecycle_event_callback_thunk(MirConnection* /*connection*/, MirLifecycleState state, void *context)
--{
--    msh::HostLifecycleEventListener* listener = static_cast<msh::HostLifecycleEventListener*>(context);
--    listener->lifecycle_event_occurred(state);
--}
--
  MirPixelFormat const cursor_pixel_format = mir_pixel_format_argb_8888;
  void copy_image(MirGraphicsRegion const& g, mg::CursorImage const& image)
@@ -227,6 +222,7 @@
      std::shared_ptr<msh::HostLifecycleEventListener> const& host_lifecycle_event_listener)
      : mir_connection{mir_connect_sync(host_socket.c_str(), name.c_str())},
        conf_change_callback{[]{}},
++      lifecycle_callback{[](MirLifecycleState){}},
        host_lifecycle_event_listener{host_lifecycle_event_listener},
        event_callback{[](MirEvent const&, mir::geometry::Rectangle const&){}}
+ {
@@ -241,8 +237,20 @@
      mir_connection_set_lifecycle_event_callback(
          mir_connection,
--        nested_lifecycle_event_callback_thunk,
--        std::static_pointer_cast<void>(host_lifecycle_event_listener).get());
++        [](MirConnection* /*connection*/, MirLifecycleState state, void *context)
++        {
++            mgn::MirClientHostConnection* obj = static_cast<mgn::MirClientHostConnection*>(context);
++            try
++            {
++                obj->handle_lifecycle_event(state);
++            }
++            catch(...)
++            {
++                report_exception();
++                abort();
++            }
++        },
++        static_cast<void*>(this));
      mir_connection_set_input_config_change_callback(
          mir_connection,
@@ -412,3 +420,14 @@
+ {
      event_callback(event, source_frame);
+ }
++
++void mgn::MirClientHostConnection::set_lifecycle_event_callback(std::function<void(MirLifecycleState)> const& cb)
++{
++    lifecycle_callback = cb;
++}
++
++void mgn::MirClientHostConnection::handle_lifecycle_event(MirLifecycleState state)
++{
++    host_lifecycle_event_listener->lifecycle_event_occurred(state);
++    lifecycle_callback(state);
++}
 === modified file 'src/server/graphics/nested/mir_client_host_connection.h'
 --- src/server/graphics/nested/mir_client_host_connection.h	2016-06-07 16:20:53 +0000
 +++ src/server/graphics/nested/mir_client_host_connection.h	2016-06-10 18:31:23 +0000
@@ -66,6 +66,7 @@
          int width, int height, MirPixelFormat pf, char const* name,
          MirBufferUsage usage, uint32_t output_id) override;
      void set_display_config_change_callback(std::function<void()> const& cb) override;
++    void set_lifecycle_event_callback(std::function<void(MirLifecycleState)> const& cb) override;
      void apply_display_config(MirDisplayConfiguration&) override;
      void set_cursor_image(CursorImage const& image) override;
@@ -80,12 +81,14 @@
      void set_input_event_callback(std::function<void(MirEvent const&, mir::geometry::Rectangle const&)> const& cb) override;
      void emit_input_event(MirEvent const& cb, mir::geometry::Rectangle const& source_frame) override;
++    void handle_lifecycle_event(MirLifecycleState state);
  private:
      void update_input_config(UniqueInputConfig input_config);
      std::mutex surfaces_mutex;
      MirConnection* const mir_connection;
      std::function<void()> conf_change_callback;
++    std::function<void(MirLifecycleState)> lifecycle_callback;
      std::shared_ptr<msh::HostLifecycleEventListener> const host_lifecycle_event_listener;
      std::vector<HostSurface*> surfaces;
 === modified file 'tests/include/mir/test/doubles/stub_host_connection.h'
 --- tests/include/mir/test/doubles/stub_host_connection.h	2016-06-09 13:15:34 +0000
 +++ tests/include/mir/test/doubles/stub_host_connection.h	2016-06-10 18:31:23 +0000
@@ -50,6 +50,10 @@
+     {
+     }
++    void set_lifecycle_event_callback(std::function<void(MirLifecycleState)> const&) override
++    {
++    }
++
      void apply_display_config(MirDisplayConfiguration&) override {}
      std::shared_ptr<graphics::nested::HostSurface>
 === modified file 'tests/unit-tests/graphics/nested/test_nested_display.cpp'
 --- tests/unit-tests/graphics/nested/test_nested_display.cpp	2016-05-04 20:53:03 +0000
 +++ tests/unit-tests/graphics/nested/test_nested_display.cpp	2016-06-10 18:31:23 +0000
@@ -21,8 +21,9 @@
  #include "src/server/graphics/nested/host_surface.h"
  #include "src/server/report/null/display_report.h"
  #include "mir/graphics/default_display_configuration_policy.h"
--#include "src/server/input/null_input_dispatcher.h"
++#include "mir/graphics/event_handler_register.h"
  #include "mir_display_configuration_builder.h"
++#include "mir/events/event_builders.h"
  #include "mir/test/doubles/mock_egl.h"
  #include "mir/test/doubles/mock_gl_config.h"
@@ -44,6 +45,32 @@
  namespace
+ {
++struct StubEventHandlerRegister : mg::EventHandlerRegister
++{
++public:
++    void register_signal_handler(std::initializer_list<int>, std::function<void(int)> const&) override
++    {
++    }
++
++    void register_signal_handler(std::initializer_list<int>, mir::UniqueModulePtr<std::function<void(int)>>) override
++    {
++    }
++
++    void register_fd_handler(std::initializer_list<int> fds, void const*, std::function<void(int)> const& callback) override
++    {
++        int fd = *begin(fds);
++        event_callback = [fd, callback]{callback(fd);};
++    }
++
++    void register_fd_handler(std::initializer_list<int>,
++                             void const*,
++                             mir::UniqueModulePtr<std::function<void(int)>>) override{};
++
++    void unregister_fd_handler(void const*) override{};
++
++    std::function<void()> event_callback;
++};
++
  class SingleDisplayHostConnection : public mtd::StubHostConnection
+ {
  public:
@@ -51,6 +78,23 @@
+     {
          return mt::build_trivial_configuration();
+     }
++
++    void set_lifecycle_event_callback(std::function<void(MirLifecycleState)> const& cb) override
++    {
++        lifecycle_callback = cb;
++    }
++
++    void simulate_suspend()
++    {
++        lifecycle_callback(mir_lifecycle_state_will_suspend);
++    }
++
++    void simulate_resume()
++    {
++        lifecycle_callback(mir_lifecycle_state_resumed);
++    }
++
++    std::function<void(MirLifecycleState)> lifecycle_callback;
  };
  class MockApplyDisplayConfigHostConnection : public SingleDisplayHostConnection
@@ -67,7 +111,7 @@
+     {
          auto nested_display_raw = new mgn::Display{
              platform,
--            std::make_shared<SingleDisplayHostConnection>(),
++            connection,
              mt::fake_shared(null_display_report),
              mt::fake_shared(default_conf_policy),
              gl_config};
@@ -75,13 +119,33 @@
          return std::unique_ptr<mgn::Display>{nested_display_raw};
+     }
++    std::shared_ptr<SingleDisplayHostConnection> const connection = std::make_shared<SingleDisplayHostConnection>();
      testing::NiceMock<mtd::MockEGL> mock_egl;
--    mi::NullInputDispatcher null_input_dispatcher;
      mir::report::null::DisplayReport null_display_report;
      mg::CloneDisplayConfigurationPolicy default_conf_policy;
      mtd::StubGLConfig stub_gl_config;
      std::shared_ptr<mtd::NullPlatform> null_platform{
          std::make_shared<mtd::NullPlatform>()};
++    StubEventHandlerRegister stub_event_handler_register;
++
++    MOCK_METHOD0(pause, void());
++    MOCK_METHOD0(resume, void());
++
++    void install_pause_resume_handlers(mgn::Display& display)
++    {
++        display.register_pause_resume_handlers(
++            stub_event_handler_register,
++            [this]()
++            {
++                pause();
++                return true;
++            },
++            [this]()
++            {
++                resume();
++                return true;
++            });
++    }
  };
+ }
@@ -243,3 +307,40 @@
      auto dummy_context = nested_display->create_gl_context();
+ }
++
++TEST_F(NestedDisplay, pauses_server_on_suspend)
++{
++    using namespace testing;
++
++    auto const nested_display = create_nested_display(
++        null_platform,
++        mt::fake_shared(stub_gl_config));
++
++    install_pause_resume_handlers(*nested_display);
++
++    EXPECT_CALL(*this, pause());
++    connection->simulate_suspend();
++    stub_event_handler_register.event_callback();
++}
++
++TEST_F(NestedDisplay, resumes_server_on_lifecycle_resume)
++{
++    using namespace testing;
++
++    auto const nested_display = create_nested_display(
++        null_platform,
++        mt::fake_shared(stub_gl_config));
++
++    install_pause_resume_handlers(*nested_display);
++
++    EXPECT_CALL(*this, pause());
++    EXPECT_CALL(*this, resume());
++
++    connection->simulate_suspend();
++    stub_event_handler_register.event_callback();
++
++    nested_display->pause();
++
++    connection->simulate_resume();
++    stub_event_handler_register.event_callback();
++}