Merge lp:~andreas-pokorny/qtmir/keep-mir-types-alive into lp:qtmir

[Andreas Pokorny]

Store the relevant mir server parts as shared_ptr and reset before shutdown

This adjustment is required for mir 0.27 and newer.

This is needed for mir-0.27 or mir-1.0 and is currently part of the dnd integration silo.

Backstory: To resolve a race between main thread to session communication and input thread to window/session communication, the internal InputDeviceObserver are executed inside the input thread, but to keep thread usage intact for 3rd party users a wrapper was added that would ensure that all external InputDeviceObservers are executed in the main thread just like before. Since there are not external users inside mir server and since there is only a weak_ptr in qtmir, the wrapper is not kept alive. So no InputDeviceHub for qtmir.

This change will also work with mir-0.26

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:626
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/644/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4774
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4802
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4625/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/644/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I don't understand the explanation.

Why do *all* these objects need preserving after Mir is done with them but need releasing before Mir releases them.

review: Needs Information

[Gerry Boland (gerboland)]

I expect this branch would be a better solution for this issue:
https://code.launchpad.net/~gerboland/qtmir/small-hooks-change/+merge/321743
Andreas - can you have a look?

[Andreas Pokorny (andreas-pokorny)]

> I expect this branch would be a better solution for this issue:
> https://code.launchpad.net/~gerboland/qtmir/small-hooks-change/+merge/321743
> Andreas - can you have a look?

This MP will show the same problem because nothing will keep the input device hub alive..

[Alan Griffiths (alan-griffiths)]

Objects that need to be kept alive are typically owned in libmirserver not by downstream projects.

It seems to me that the right fix is in Mir. Mir should own the ExternalInputDeviceHub by attaching it to mir::DisplayServer::Private, or something that this owns.

review: Disapprove

[Alan Griffiths (alan-griffiths)]

> Objects that need to be kept alive are typically owned in libmirserver not by
> downstream projects.
>
> It seems to me that the right fix is in Mir. Mir should own the
> ExternalInputDeviceHub by attaching it to mir::DisplayServer::Private, or
> something that this owns.

For example: lp:~alan-griffiths/mir/preserve-ExternalInputDeviceHub/+merge/321845

Unmerged revisions

626. By Andreas Pokorny on 2017-03-28

Store the relevant mir server parts as shared_ptr and reset before shutdown

=== modified file 'src/platforms/mirserver/mirserverhooks.cpp'
--- src/platforms/mirserver/mirserverhooks.cpp	2017-03-13 16:49:49 +0000
+++ src/platforms/mirserver/mirserverhooks.cpp	2017-03-30 09:15:23 +0000
@@ -70,11 +70,11 @@
 
 struct qtmir::MirServerHooks::Self
 {
-    std::weak_ptr<PromptSessionListener> m_promptSessionListener;
-    std::weak_ptr<mir::graphics::Display> m_mirDisplay;
-    std::weak_ptr<mir::shell::DisplayConfigurationController> m_mirDisplayConfigurationController;
-    std::weak_ptr<mir::scene::PromptSessionManager> m_mirPromptSessionManager;
-    std::weak_ptr<mir::input::InputDeviceHub> m_inputDeviceHub;
+    std::shared_ptr<PromptSessionListener> m_promptSessionListener;
+    std::shared_ptr<mir::graphics::Display> m_mirDisplay;
+    std::shared_ptr<mir::shell::DisplayConfigurationController> m_mirDisplayConfigurationController;
+    std::shared_ptr<mir::scene::PromptSessionManager> m_mirPromptSessionManager;
+    std::shared_ptr<mir::input::InputDeviceHub> m_inputDeviceHub;
 };
 
 qtmir::MirServerHooks::MirServerHooks() :
@@ -104,36 +104,44 @@
             self->m_mirPromptSessionManager = server.the_prompt_session_manager();
             self->m_inputDeviceHub = server.the_input_device_hub();
         });
+
+    server.add_stop_callback([this]
+        {
+            self->m_mirDisplay.reset();
+            self->m_mirDisplayConfigurationController.reset();
+            self->m_mirPromptSessionManager.reset();
+            self->m_inputDeviceHub.reset();
+        });
 }
 
 PromptSessionListener *qtmir::MirServerHooks::promptSessionListener() const
 {
-    if (auto result = self->m_promptSessionListener.lock())
-        return result.get();
+    if (self->m_promptSessionListener)
+        return self->m_promptSessionListener.get();
 
     throw std::logic_error("No prompt session listener available. Server not running?");
 }
 
 std::shared_ptr<mir::scene::PromptSessionManager> qtmir::MirServerHooks::thePromptSessionManager() const
 {
-    if (auto result = self->m_mirPromptSessionManager.lock())
-        return result;
+    if (self->m_mirPromptSessionManager)
+        return self->m_mirPromptSessionManager;
 
     throw std::logic_error("No prompt session manager available. Server not running?");
 }
 
 std::shared_ptr<mir::graphics::Display> qtmir::MirServerHooks::theMirDisplay() const
 {
-    if (auto result = self->m_mirDisplay.lock())
-        return result;
+    if (self->m_mirDisplay)
+        return self->m_mirDisplay;
 
     throw std::logic_error("No display available. Server not running?");
 }
 
 std::shared_ptr<mir::input::InputDeviceHub> qtmir::MirServerHooks::theInputDeviceHub() const
 {
-    if (auto result = self->m_inputDeviceHub.lock())
-        return result;
+    if (self->m_inputDeviceHub)
+        return self->m_inputDeviceHub;
 
     throw std::logic_error("No input device hub available. Server not running?");
 }
@@ -141,7 +149,7 @@
 QSharedPointer<ScreensController> qtmir::MirServerHooks::createScreensController(QSharedPointer<ScreensModel> const &screensModel) const
 {
     return QSharedPointer<ScreensController>(
-        new ScreensController(screensModel, theMirDisplay(), self->m_mirDisplayConfigurationController.lock()));
+        new ScreensController(screensModel, theMirDisplay(), self->m_mirDisplayConfigurationController));
 }
 
 void qtmir::MirServerHooks::createInputDeviceObserver()