Commit eb6e2619 authored by Jiří Paleček's avatar Jiří Paleček Committed by David Edmundson
Browse files

Sanitize signal handling in ksmserver

Summary:
The TERM signal handling in ksmserver invokes functions which are not async-signal safe, like Qt functions and C++ destructors. Moreover, the signal handling can occur in other than the main thread, which leads to Qt complaining about  functions being invoked from the wrong thread. Such a crash can be seen in a report of [[ bug 384316 |https://bugs.kde.org/show_bug.cgi?id=384316 ]].

To fix both of these issues, this change makes the signal handling use the self-pipe trick, which signals the need for termination to the main thread by writing to a special-purpose file descriptor. The main loop then takes care of the termination. This is mostly inspired by [[ http://doc.qt.io/qt-5/unix-signals.html ]].

Note that `QApplication::quit` already does what we need when destroying the server, particularly deleting `the_server`/calling `cleanUp()`.

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: broulik, graesslin, davidedmundson, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D8673
parent 28ddeb27
......@@ -524,6 +524,7 @@ void KSMServer::setupXIOErrorHandler()
XSetIOErrorHandler(Xio_ErrorHandler);
}
static int wake_up_socket = -1;
static void sighandler(int sig)
{
if (sig == SIGHUP) {
......@@ -531,17 +532,8 @@ static void sighandler(int sig)
return;
}
if (the_server)
{
KSMServer *server = the_server;
the_server = nullptr;
server->cleanUp();
delete server;
}
if (qApp)
qApp->quit();
//::exit(0);
char ch = 0;
(void)::write(wake_up_socket, &ch, 1);
}
......@@ -607,6 +599,7 @@ KSMServer::KSMServer( const QString& windowManager, InitFlags flags )
: wmProcess( nullptr )
, sessionGroup( QStringLiteral( "" ) )
, logoutEffectWidget( nullptr )
, sockets{ -1, -1 }
{
if (!flags.testFlag(InitFlag::NoLockScreen)) {
ScreenLocker::KSldApp::self()->initialize();
......@@ -615,6 +608,12 @@ KSMServer::KSMServer( const QString& windowManager, InitFlags flags )
}
}
if(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets) != 0)
qFatal("Could not create socket pair, error %d (%s)", errno, strerror(errno));
wake_up_socket = sockets[0];
QSocketNotifier* n = new QSocketNotifier(sockets[1], QSocketNotifier::Read, this);
qApp->connect(n, &QSocketNotifier::activated, &QApplication::quit);
new KSMServerInterfaceAdaptor( this );
QDBusConnection::sessionBus().registerObject(QStringLiteral("/KSMServer"), this);
kcminitSignals = nullptr;
......@@ -738,6 +737,12 @@ void KSMServer::cleanUp()
clean = true;
IceFreeListenObjs (numTransports, listenObjs);
wake_up_socket = -1;
::close(sockets[1]);
::close(sockets[0]);
sockets[0] = -1;
sockets[1] = -1;
QByteArray fName = QFile::encodeName(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation) + QLatin1Char('/') + QStringLiteral("KSMserver"));
QString display = QString::fromLocal8Bit(::getenv("DISPLAY"));
// strip the screen number from the display
......
......@@ -296,6 +296,7 @@ private:
QList<KSMClient*> clientsToKill;
QList<KSMClient*> clientsToSave;
int sockets[2];
friend bool readFromPipe(int pipe);
};
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment