Merge lp:~unity-api-team/hud/unix-signal-handler into lp:hud/14.04

Proposed by Pete Woods
Status: Merged
Approved by: Pete Woods
Approved revision: 392
Merged at revision: 385
Proposed branch: lp:~unity-api-team/hud/unix-signal-handler
Merge into: lp:hud/14.04
Prerequisite: lp:~unity-api-team/hud/test-failures
Diff against target: 233 lines (+164/-10)
5 files modified
service/CMakeLists.txt (+1/-0)
service/ItemStore.cpp (+1/-1)
service/SignalHandler.cpp (+99/-0)
service/SignalHandler.h (+60/-0)
service/main.cpp (+3/-9)
To merge this branch: bzr merge lp:~unity-api-team/hud/unix-signal-handler
Reviewer Review Type Date Requested Status
Marcus Tomlinson (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+212393@code.launchpad.net

Commit message

Call only "safe" write method from UNIX signal handler

See http://pubs.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_04.html#tag_02_04_01

Description of the change

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
  * Yes
 * Did you build your software in a clean sbuild/pbuilder chroot or ppa?
  * Yes
 * Did you build your software in a clean sbuild/pbuilder armhf chroot or ppa?
  * Yes
 * Has your component "TestPlan” been executed successfully on emulator, N4?
  * Yes
 * Has a 5 minute exploratory testing run been executed on N4?
  * Yes
 * If you changed the packaging (debian), did you subscribe a core-dev to this MP?
  * N/A
 * If you changed the UI, did you subscribe the design-reviewers to this MP?
  * No change
 * What components might get impacted by your changes?
  * Unity7
  * Unity8
 * Have you requested review by the teams of these owning components?
  * Yes

Check List:
https://wiki.ubuntu.com/Process/Merges/Checklists/hud

Test Plan:
https://wiki.ubuntu.com/Process/Merges/TestPlan/hud

Silo:
<waiting for silo>

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
390. By Pete Woods

Split whitespace / semicolon regular expression

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
391. By Pete Woods

Responsd to review comments

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

One more pedantic style fix ;)

90 + struct sigaction sigint, term;

should be:

90 + struct sigaction sigint, sigterm;

review: Needs Fixing
392. By Pete Woods

Rename term -> sigterm

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Cools +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'service/CMakeLists.txt'
2--- service/CMakeLists.txt 2014-02-15 10:23:45 +0000
3+++ service/CMakeLists.txt 2014-03-25 09:43:25 +0000
4@@ -29,6 +29,7 @@
5 QueryImpl.cpp
6 Result.cpp
7 SearchSettings.cpp
8+ SignalHandler.cpp
9 SqliteUsageTracker.cpp
10 UsageTracker.cpp
11 Voice.cpp
12
13=== modified file 'service/ItemStore.cpp'
14--- service/ItemStore.cpp 2014-03-25 09:43:25 +0000
15+++ service/ItemStore.cpp 2014-03-25 09:43:25 +0000
16@@ -31,7 +31,7 @@
17 static const QRegularExpression SINGLE_AMPERSAND("(?<![&])[&](?![&])");
18 static const QRegularExpression BAD_CHARACTERS("\\.\\.\\.|…");
19 static const QRegularExpression WHITESPACE("\\s+");
20-static const QRegularExpression WHITESPACE_OR_SEMICOLON("[;\\s+]");
21+static const QRegularExpression WHITESPACE_OR_SEMICOLON("[;\\s]+");
22
23 ItemStore::ItemStore(const QString &applicationId,
24 UsageTracker::Ptr usageTracker, SearchSettings::Ptr settings) :
25
26=== added file 'service/SignalHandler.cpp'
27--- service/SignalHandler.cpp 1970-01-01 00:00:00 +0000
28+++ service/SignalHandler.cpp 2014-03-25 09:43:25 +0000
29@@ -0,0 +1,99 @@
30+/*
31+ * Copyright (C) 2014 Canonical, Ltd.
32+ *
33+ * This program is free software: you can redistribute it and/or modify it
34+ * under the terms of the GNU General Public License version 3, as published
35+ * by the Free Software Foundation.
36+ *
37+ * This program is distributed in the hope that it will be useful, but
38+ * WITHOUT ANY WARRANTY; without even the implied warranties of
39+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
40+ * PURPOSE. See the GNU General Public License for more details.
41+ *
42+ * You should have received a copy of the GNU General Public License along
43+ * with this program. If not, see <http://www.gnu.org/licenses/>.
44+ *
45+ * Author: Pete Woods <pete.woods@canonical.com>
46+ */
47+
48+#include <service/SignalHandler.h>
49+
50+#include <QCoreApplication>
51+#include <QDebug>
52+
53+#include <csignal>
54+#include <sys/socket.h>
55+#include <unistd.h>
56+
57+using namespace hud::service;
58+
59+int SignalHandler::sigintFd[2];
60+
61+int SignalHandler::sigtermFd[2];
62+
63+SignalHandler::SignalHandler(QObject *parent) :
64+ QObject(parent) {
65+
66+ if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sigintFd)) {
67+ qFatal("Couldn't create INT socketpair");
68+ }
69+ if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sigtermFd)) {
70+ qFatal("Couldn't create TERM socketpair");
71+ }
72+
73+ m_socketNotifierInt = new QSocketNotifier(sigintFd[1], QSocketNotifier::Read, this);
74+ connect(m_socketNotifierInt, &QSocketNotifier::activated, this, &SignalHandler::handleSigInt);
75+ m_socketNotifierTerm = new QSocketNotifier(sigtermFd[1], QSocketNotifier::Read, this);
76+ connect(m_socketNotifierTerm, &QSocketNotifier::activated, this, &SignalHandler::handleSigTerm);
77+}
78+
79+void SignalHandler::intSignalHandler(int) {
80+ char a = 1;
81+ ::write(sigintFd[0], &a, sizeof(a));
82+}
83+
84+void SignalHandler::termSignalHandler(int) {
85+ char a = 1;
86+ ::write(sigtermFd[0], &a, sizeof(a));
87+}
88+
89+int SignalHandler::setupUnixSignalHandlers() {
90+ struct sigaction sigint, sigterm;
91+
92+ sigint.sa_handler = SignalHandler::intSignalHandler;
93+ sigemptyset(&sigint.sa_mask);
94+ sigint.sa_flags = 0;
95+ sigint.sa_flags |= SA_RESTART;
96+
97+ if (sigaction(SIGINT, &sigint, 0) > 0)
98+ return 1;
99+
100+ sigterm.sa_handler = SignalHandler::termSignalHandler;
101+ sigemptyset(&sigterm.sa_mask);
102+ sigterm.sa_flags |= SA_RESTART;
103+
104+ if (sigaction(SIGTERM, &sigterm, 0) > 0)
105+ return 2;
106+
107+ return 0;
108+}
109+
110+void SignalHandler::handleSigTerm() {
111+ m_socketNotifierTerm->setEnabled(false);
112+ char tmp;
113+ ::read(sigtermFd[1], &tmp, sizeof(tmp));
114+
115+ QCoreApplication::exit(0);
116+
117+ m_socketNotifierTerm->setEnabled(true);
118+}
119+
120+void SignalHandler::handleSigInt() {
121+ m_socketNotifierInt->setEnabled(false);
122+ char tmp;
123+ ::read(sigintFd[1], &tmp, sizeof(tmp));
124+
125+ QCoreApplication::exit(0);
126+
127+ m_socketNotifierInt->setEnabled(true);
128+}
129
130=== added file 'service/SignalHandler.h'
131--- service/SignalHandler.h 1970-01-01 00:00:00 +0000
132+++ service/SignalHandler.h 2014-03-25 09:43:25 +0000
133@@ -0,0 +1,60 @@
134+/*
135+ * Copyright (C) 2014 Canonical, Ltd.
136+ *
137+ * This program is free software: you can redistribute it and/or modify it
138+ * under the terms of the GNU General Public License version 3, as published
139+ * by the Free Software Foundation.
140+ *
141+ * This program is distributed in the hope that it will be useful, but
142+ * WITHOUT ANY WARRANTY; without even the implied warranties of
143+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
144+ * PURPOSE. See the GNU General Public License for more details.
145+ *
146+ * You should have received a copy of the GNU General Public License along
147+ * with this program. If not, see <http://www.gnu.org/licenses/>.
148+ *
149+ * Author: Pete Woods <pete.woods@canonical.com>
150+ */
151+
152+#ifndef HUD_SERVICE_SIGNALHANDLER_H_
153+#define HUD_SERVICE_SIGNALHANDLER_H_
154+
155+#include <QObject>
156+#include <QSocketNotifier>
157+
158+namespace hud {
159+namespace service {
160+
161+class SignalHandler: public QObject {
162+Q_OBJECT
163+
164+public:
165+ SignalHandler(QObject *parent = 0);
166+
167+ ~SignalHandler() = default;
168+
169+ static int setupUnixSignalHandlers();
170+
171+protected Q_SLOTS:
172+ void handleSigInt();
173+
174+ void handleSigTerm();
175+
176+protected:
177+ static void intSignalHandler(int unused);
178+
179+ static void termSignalHandler(int unused);
180+
181+ static int sigintFd[2];
182+
183+ static int sigtermFd[2];
184+
185+ QSocketNotifier *m_socketNotifierInt;
186+
187+ QSocketNotifier *m_socketNotifierTerm;
188+};
189+
190+}
191+}
192+
193+#endif /* HUD_SERVICE_SIGNALHANDLER_H_ */
194
195=== modified file 'service/main.cpp'
196--- service/main.cpp 2013-11-27 14:28:43 +0000
197+++ service/main.cpp 2014-03-25 09:43:25 +0000
198@@ -18,19 +18,14 @@
199
200 #include <common/Localisation.h>
201 #include <service/Factory.h>
202+#include <service/SignalHandler.h>
203
204 #include <QDebug>
205 #include <QApplication>
206-#include <csignal>
207
208 using namespace std;
209 using namespace hud::service;
210
211-static void exitQt(int sig) {
212- Q_UNUSED(sig);
213- QApplication::exit(0);
214-}
215-
216 int main(int argc, char *argv[]) {
217 qputenv("QT_QPA_PLATFORM", "minimal");
218 QApplication application(argc, argv);
219@@ -39,12 +34,11 @@
220 bindtextdomain(GETTEXT_PACKAGE, GNOMELOCALEDIR);
221 textdomain(GETTEXT_PACKAGE);
222
223- signal(SIGINT, &exitQt);
224- signal(SIGTERM, &exitQt);
225-
226 try {
227 Factory factory;
228 factory.singletonHudService();
229+ SignalHandler handler;
230+ handler.setupUnixSignalHandlers();
231 return application.exec();
232 } catch (std::exception &e) {
233 qWarning() << _("Hud Service:") << e.what();

Subscribers

People subscribed via source and target branches