Merge lp:~gerboland/unity-mir/fix-shutdown-crash2 into lp:unity-mir

[Gerry Boland]

ApplicationManager instance is owned & managed by the QML engine. Wrapping it with QSharedPointer causes shutdown crash due to double deletion. Resolve by removing the shared pointer wrapping.

I tried specifying C++ ownership of the ApplicationManager object to the QmlEngine, but it did not work.

Fixes shutdown crash of unity8 in latest developer image.

To reproduce the crash, in adb, simply run
  kill -SIGTERM `pidof unity8`

With this patch, shutdown is correct - just note that shutdown takes several seconds.

• Are there any related MPs required for this MP to build/function as expected? Please list.
No
• Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
• If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:196
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/288/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/152
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/152/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/151

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/288/rebuild

review: Approve (continuous-integration)

[Michał Sawicz (saviq)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes.

 * Did CI run pass? If not, please explain why.
Yes.

review: Approve

[lp:~gerboland/unity-mir/fix-shutdown-crash2]

197. By Gerry Boland on 2014-03-11

Add big fixme

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:197
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/289/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/152
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/153
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/153/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/152

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/289/rebuild

review: Approve (continuous-integration)

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2014-03-11 03:40:49 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2014-03-11 15:36:35 +0000
@@ -116,13 +116,13 @@
 }
 }
 
-QSharedPointer<ApplicationManager> ApplicationManager::Factory::Factory::create()
+ApplicationManager* ApplicationManager::Factory::Factory::create()
 {
     QMirServerApplication* mirServerApplication = dynamic_cast<QMirServerApplication*>(QCoreApplication::instance());
     if (mirServerApplication == nullptr) {
         LOG("Need to use QMirServerApplication");
         QCoreApplication::quit();
-        return QSharedPointer<ApplicationManager>(nullptr);
+        return nullptr;
     }
 
     ShellServerConfiguration * mirServer = mirServerApplication->server();
@@ -133,33 +133,36 @@
     QSharedPointer<TaskController> taskController(new TaskController(nullptr, appController));
     QSharedPointer<DesktopFileReader::Factory> fileReaderFactory(new DesktopFileReader::Factory());
     QSharedPointer<ProcInfo> procInfo(new ProcInfo());
-    QSharedPointer<ApplicationManager> appManager(
-        new ApplicationManager(
-            taskController,
-            fileReaderFactory,
-            procInfo,
-            mirServer->the_focus_controller(),
-            displaySize
-            )
-        );
-
-
-    connectToSessionListener(appManager.data(), mirServer->sessionListener());
-    connectToSessionAuthorizer(appManager.data(), mirServer->sessionAuthorizer());
-    connectToPlacementStrategy(appManager.data(), mirServer->placementStrategy());
-    connectToTaskController(appManager.data(), taskController.data());
+
+    // FIXME: We should use a QSharedPointer to wrap this ApplicationManager object, which requires us
+    // to use the data() method to pass the raw pointer to the QML engine. However the QML engine appears
+    // to take ownership of the object, and deletes it when it wants to. This conflicts with the purpose
+    // of the QSharedPointer, and a double-delete results. Trying QQmlEngine::setObjectOwnership on the 
+    // object no effect, which it should. Need to investigate why.
+    ApplicationManager* appManager = new ApplicationManager(
+                                             taskController,
+                                             fileReaderFactory,
+                                             procInfo,
+                                             mirServer->the_focus_controller(),
+                                             displaySize
+                                         );
+
+    connectToSessionListener(appManager, mirServer->sessionListener());
+    connectToSessionAuthorizer(appManager, mirServer->sessionAuthorizer());
+    connectToPlacementStrategy(appManager, mirServer->placementStrategy());
+    connectToTaskController(appManager, taskController.data());
 
     return appManager;
 }
 
 ApplicationManager* ApplicationManager::singleton()
 {
-    static QSharedPointer<ApplicationManager> instance;
+    static ApplicationManager* instance;
     if (!instance) {
         Factory appFactory;
         instance = appFactory.create();
     }
-    return instance.data();
+    return instance;
 }
 
 ApplicationManager::ApplicationManager(

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2014-03-11 03:40:26 +0000
+++ src/modules/Unity/Application/application_manager.h	2014-03-11 15:36:35 +0000
@@ -63,7 +63,7 @@
     class Factory
     {
     public:
-        QSharedPointer<ApplicationManager> create();
+        ApplicationManager* create();
     };
     // Mapping enums to Ubuntu Platform API enums.
     enum Flag {