Merge lp:~gerboland/unity-mir/shutdown-crash-fix3 into lp:unity-mir/devel-mir-next

Proposed by Gerry Boland
Status: Merged
Approved by: Michał Sawicz
Approved revision: 223
Merged at revision: 220
Proposed branch: lp:~gerboland/unity-mir/shutdown-crash-fix3
Merge into: lp:unity-mir/devel-mir-next
Diff against target: 62 lines (+22/-3)
1 file modified
src/unity-mir/qmirserver.cpp (+22/-3)
To merge this branch: bzr merge lp:~gerboland/unity-mir/shutdown-crash-fix3
Reviewer Review Type Date Requested Status
Michał Sawicz Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+218394@code.launchpad.net

Commit message

Fix crash on Mir-initiated shutdown, where stop() was being called on an already shutting-down server.

Mir initiates shutdown on SIGINT & SIGTERM, need to distinguish that shutdown from a client-initiated shutdown. Do this by installing a custom signal handler that is run after Mir's initiate-shutdown handler is called, so that we only call server.stop() on a client-initiated shutdown.

Description of the change

Fix crash on Mir-initiated shutdown, where stop() was being called on an already shutting-down server

To test, need to cover 2 use-cases:
1. Mir-initiated shutdown: run unity8 manually in shell, then hit Ctrl+C - should not crash
2. Qt-initiated shutdown: the first-run wizard, when it has completed, calls Qt.quit() on itself. This should cleanly shutdown the process.

To post a comment you must log in.
221. By Gerry Boland

bump

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
222. By Gerry Boland

Missing csignal header

223. By Gerry Boland

Add Mir main_loop.h header

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Yeah, tested both unity8 and the wizard and things are better.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/unity-mir/qmirserver.cpp'
--- src/unity-mir/qmirserver.cpp 2014-04-09 15:47:05 +0000
+++ src/unity-mir/qmirserver.cpp 2014-05-06 12:35:11 +0000
@@ -19,6 +19,7 @@
19#include <mir/abnormal_exit.h>19#include <mir/abnormal_exit.h>
20#include <mir/default_server_configuration.h>20#include <mir/default_server_configuration.h>
21#include <mir/display_server.h>21#include <mir/display_server.h>
22#include <mir/main_loop.h>
2223
23// Platform API24// Platform API
24#include <application/ubuntu_application_api_mirserver_priv.h>25#include <application/ubuntu_application_api_mirserver_priv.h>
@@ -30,9 +31,11 @@
3031
31// Std32// Std
32#include <chrono>33#include <chrono>
34#include <csignal>
33#include <thread>35#include <thread>
3436
35// local37// local
38#include "logging.h"
36#include "qmirserver.h"39#include "qmirserver.h"
3740
38QMirServer::QMirServer(int argc, const char* argv[], QObject *parent)41QMirServer::QMirServer(int argc, const char* argv[], QObject *parent)
@@ -53,19 +56,35 @@
53 auto argv = m_argv;56 auto argv = m_argv;
54 auto config = new ShellServerConfiguration(m_argc, m_argv);57 auto config = new ShellServerConfiguration(m_argc, m_argv);
55 std::thread *t;58 std::thread *t;
59 bool mirServerShutDown = false;
5660
57 mir::run_mir(*config, [config, &client, &argc, &argv, &t](mir::DisplayServer& server) {61 mir::run_mir(*config, [&](mir::DisplayServer &server) {
58 ua_ui_mirserver_init(*config);62 ua_ui_mirserver_init(*config);
5963
64 // Mir initiates shutdown on SIGINT & SIGTERM, need to distinguish that shutdown from a
65 // client-initiated shutdown. Do this by installing a custom signal handler that is run
66 // after Mir's initiate-shutdown handler is called.
67 config->the_main_loop()->register_signal_handler(
68 {SIGINT, SIGTERM},
69 [&](int)
70 {
71 DLOG("Mir-initiated shut down in progress");
72 mirServerShutDown = true;
73 });
74
60 try {75 try {
61 t = new std::thread([&]() {76 t = new std::thread([&]() {
62 client (argc, argv, config);77 client (argc, argv, config);
63 server.stop();78 if (!mirServerShutDown) {
79 DLOG("Qt-initiated shut down in progress");
80 server.stop();
81 }
64 });82 });
65 } catch (...) {83 } catch (...) {
66 qDebug() << "Exception caught, quitting";84 LOG("Exception caught, quitting");
67 }85 }
68 });86 });
87
69 if (QCoreApplication::instance()) {88 if (QCoreApplication::instance()) {
70 bool aboutToQuitSignaled = false;89 bool aboutToQuitSignaled = false;
71 QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, [&]() {90 QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, [&]() {

Subscribers

People subscribed via source and target branches