Fix up signal handling in QFbVtHandler

Start using signalfd where we can. Drop the crash (SIGSEGV, SIGBUS) handling completely.

The crash handling that was in place previously was not async-safe. It also prevented getting
a core dump. So just remove it. There is no safe solution for a single application process since
restoring the keyboard, video modes, etc. all need unsafe calls in the signal handler almost for sure.

We can however improve the handling of non-crash scenarios greatly:

Introduce support for SIGINT, allowing nicely and cleanly restoring the video
mode with the KMS backend when pressing Ctrl+C while QT_QPA_ENABLE_TERMINAL_KEYBOARD
is set.

Same goes for keyboard suspend (SIGTSTP, Ctrl+Z). When QT_QPA_ENABLE_TERMINAL_KEYBOARD is set,
platform plugins now have the possibility to act upon Ctrl+Z. As an example eglfs' KMS backend
is enhanced to handle this by restoring the video mode before suspending the process, and
reinitializing when brought into foreground again (SIGCONT).

SIGTERM is also handled. This is extremely handy when starting an application locally on the embedded
device and then kill-ing it via a remote ssh session. Keyboard and video mode is now cleanly restored.

Finally, when disabling the keyboard, try setting also KDSKBMUTE.

Change-Id: I2b3608dc23c798e2b39f74cb27f12dcb0e958435
Reviewed-by: Louai Al-Khanji <louai.al-khanji@theqtcompany.com>

src/platformsupport/eglconvenience/qeglplatformintegration_p.h

@@ -90,6 +90,8 @@ public:
 
     QFunctionPointer platformFunction(const QByteArray &function) const Q_DECL_OVERRIDE;
 
+    QFbVtHandler *vtHandler() { return m_vtHandler.data(); }
+
 protected:
     virtual QEGLPlatformWindow *createWindow(QWindow *window) const = 0;
     virtual QEGLPlatformContext *createContext(const QSurfaceFormat &format,

src/platformsupport/fbconvenience/qfbvthandler.cpp

@@ -32,72 +32,148 @@
 ****************************************************************************/
 
 #include "qfbvthandler_p.h"
-#include <QtCore/private/qcrashhandler_p.h>
-#include <QtGui/private/qguiapplication_p.h>
+#include <QtCore/QSocketNotifier>
 
-#if defined(Q_OS_LINUX) && !defined(QT_NO_EVDEV)
-#define HAS_VT
-#endif
+#if defined(Q_OS_LINUX) && !defined(Q_OS_ANDROID) && (!defined(QT_NO_EVDEV) || !defined(QT_NO_LIBINPUT))
 
-#ifdef HAS_VT
+#define VTH_ENABLED
 
+#include <unistd.h>
+#include <signal.h>
+#include <errno.h>
+#include <sys/signalfd.h>
 #include <sys/ioctl.h>
 #include <linux/kd.h>
 
+#ifndef KDSKBMUTE
+#define KDSKBMUTE 0x4B51
+#endif
+
 #ifdef K_OFF
 #define KBD_OFF_MODE K_OFF
 #else
 #define KBD_OFF_MODE K_RAW
 #endif
 
-#endif // HAS_VT
+#endif
 
 QT_BEGIN_NAMESPACE
 
-QFbVtHandler *QFbVtHandler::self = 0;
-
 QFbVtHandler::QFbVtHandler(QObject *parent)
-    : QObject(parent), m_tty(-1)
+    : QObject(parent),
+      m_tty(-1),
+      m_signalFd(-1),
+      m_signalNotifier(0)
 {
-    Q_ASSERT(!self);
-    self = this;
+#ifdef VTH_ENABLED
+    if (isatty(0)) {
+        m_tty = 0;
+        ioctl(m_tty, KDGKBMODE, &m_oldKbdMode);
 
-#ifdef HAS_VT
-    if (!isatty(0))
-        return;
+        if (!qEnvironmentVariableIntValue("QT_QPA_ENABLE_TERMINAL_KEYBOARD")) {
+            // Disable the tty keyboard.
+            ioctl(m_tty, KDSKBMUTE, 1);
+            ioctl(m_tty, KDSKBMODE, KBD_OFF_MODE);
+        }
+    }
 
-    m_tty = 0;
-    ::ioctl(m_tty, KDGKBMODE, &m_oldKbdMode);
-    if (!qEnvironmentVariableIntValue("QT_QPA_ENABLE_TERMINAL_KEYBOARD")) {
-        ::ioctl(m_tty, KDSKBMODE, KBD_OFF_MODE);
-        QGuiApplicationPrivate *appd = QGuiApplicationPrivate::instance();
-        Q_ASSERT(appd);
-        QSegfaultHandler::initialize(appd->argv, appd->argc);
-        QSegfaultHandler::installCrashHandler(crashHandler);
+    // SIGSEGV and such cannot safely be blocked. We cannot handle them in an
+    // async-safe manner either. Restoring the keyboard, video mode, etc. may
+    // all contain calls that cannot safely be made from a signal handler.
+
+    // Other signals: block them and use signalfd.
+    sigset_t mask;
+    sigemptyset(&mask);
+
+    // Catch Ctrl+C.
+    sigaddset(&mask, SIGINT);
+
+    // Ctrl+Z. Up to the platform plugins to handle it in a meaningful way.
+    sigaddset(&mask, SIGTSTP);
+    sigaddset(&mask, SIGCONT);
+
+    // Default signal used by kill. To overcome the common issue of no cleaning
+    // up when killing a locally started app via a remote session.
+    sigaddset(&mask, SIGTERM);
+
+    m_signalFd = signalfd(-1, &mask, SFD_CLOEXEC);
+    if (m_signalFd < 0) {
+        qErrnoWarning(errno, "signalfd() failed");
+    } else {
+        m_signalNotifier = new QSocketNotifier(m_signalFd, QSocketNotifier::Read, this);
+        connect(m_signalNotifier, &QSocketNotifier::activated, this, &QFbVtHandler::handleSignal);
+
+        // Block the signals that are handled via signalfd. Applies only to the current
+        // thread, but new threads will inherit the creator's signal mask.
+        pthread_sigmask(SIG_BLOCK, &mask, 0);
     }
 #endif
 }
 
 QFbVtHandler::~QFbVtHandler()
 {
-    self->cleanup();
-    self = 0;
-}
+#ifdef VTH_ENABLED
+    restoreKeyboard();
 
-void QFbVtHandler::cleanup()
-{
-    if (m_tty == -1)
-        return;
-
-#ifdef HAS_VT
-    ::ioctl(m_tty, KDSKBMODE, m_oldKbdMode);
+    if (m_signalFd != -1)
+        close(m_signalFd);
 #endif
 }
 
-void QFbVtHandler::crashHandler()
+void QFbVtHandler::restoreKeyboard()
 {
-    Q_ASSERT(self);
-    self->cleanup();
+#ifdef VTH_ENABLED
+    if (m_tty == -1)
+        return;
+
+    ioctl(m_tty, KDSKBMUTE, 0);
+    ioctl(m_tty, KDSKBMODE, m_oldKbdMode);
+#endif
+}
+
+// To be called from the slot connected to suspendRequested() in case the
+// platform plugin does in fact allow suspending on Ctrl+Z.
+void QFbVtHandler::suspend()
+{
+#ifdef VTH_ENABLED
+    kill(getpid(), SIGSTOP);
+#endif
+}
+
+void QFbVtHandler::handleSignal()
+{
+#ifdef VTH_ENABLED
+    m_signalNotifier->setEnabled(false);
+
+    signalfd_siginfo sig;
+    if (read(m_signalFd, &sig, sizeof(sig)) == sizeof(sig)) {
+        switch (sig.ssi_signo) {
+        case SIGINT: // fallthrough
+        case SIGTERM:
+            handleInt();
+            break;
+        case SIGTSTP:
+            emit suspendRequested();
+            break;
+        case SIGCONT:
+            emit resumed();
+            break;
+        default:
+            break;
+        }
+    }
+
+    m_signalNotifier->setEnabled(true);
+#endif
+}
+
+void QFbVtHandler::handleInt()
+{
+#ifdef VTH_ENABLED
+    emit interrupted();
+    restoreKeyboard();
+    _exit(1);
+#endif
 }
 
 QT_END_NAMESPACE

src/platformsupport/fbconvenience/qfbvthandler_p.h

@@ -49,6 +49,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QSocketNotifier;
+
 class QFbVtHandler : public QObject
 {
     Q_OBJECT
@@ -57,13 +59,24 @@ public:
     QFbVtHandler(QObject *parent = 0);
     ~QFbVtHandler();
 
-private:
-    void cleanup();
-    static void crashHandler();
+    void suspend();
+
+signals:
+    void interrupted();
+    void suspendRequested();
+    void resumed();
+
+private slots:
+    void handleSignal();
+
+private:
+    void restoreKeyboard();
+    void handleInt();
 
-    static QFbVtHandler *self;
     int m_tty;
     int m_oldKbdMode;
+    int m_signalFd;
+    QSocketNotifier *m_signalNotifier;
 };
 
 QT_END_NAMESPACE

src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.cpp

@@ -37,10 +37,35 @@
 
 #include <QtCore/QLoggingCategory>
 
+#include <QtGui/private/qguiapplication_p.h>
+#include <QtPlatformSupport/private/qeglplatformintegration_p.h>
+#include <QtPlatformSupport/private/qfbvthandler_p.h>
+
 QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(qLcEglfsKmsDebug)
 
+class QEglFSKmsInterruptHandler : public QObject
+{
+public:
+    QEglFSKmsInterruptHandler(QEglFSKmsScreen *screen) : m_screen(screen) {
+        m_vtHandler = static_cast<QEGLPlatformIntegration *>(QGuiApplicationPrivate::platformIntegration())->vtHandler();
+        connect(m_vtHandler, &QFbVtHandler::interrupted, this, &QEglFSKmsInterruptHandler::restoreVideoMode);
+        connect(m_vtHandler, &QFbVtHandler::suspendRequested, this, &QEglFSKmsInterruptHandler::handleSuspendRequest);
+    }
+
+public slots:
+    void restoreVideoMode() { m_screen->restoreMode(); }
+    void handleSuspendRequest() {
+        m_screen->restoreMode();
+        m_vtHandler->suspend();
+    }
+
+private:
+    QFbVtHandler *m_vtHandler;
+    QEglFSKmsScreen *m_screen;
+};
+
 void QEglFSKmsScreen::bufferDestroyedHandler(gbm_bo *bo, void *data)
 {
     FrameBuffer *fb = static_cast<FrameBuffer *>(data);
@@ -93,12 +118,18 @@ QEglFSKmsScreen::QEglFSKmsScreen(QEglFSKmsIntegration *integration,
     , m_output(output)
     , m_pos(position)
     , m_cursor(Q_NULLPTR)
+    , m_interruptHandler(new QEglFSKmsInterruptHandler(this))
 {
 }
 
 QEglFSKmsScreen::~QEglFSKmsScreen()
 {
     restoreMode();
+    if (m_output.saved_crtc) {
+        drmModeFreeCrtc(m_output.saved_crtc);
+        m_output.saved_crtc = Q_NULLPTR;
+    }
+    delete m_interruptHandler;
 }
 
 QRect QEglFSKmsScreen::geometry() const
@@ -268,9 +299,6 @@ void QEglFSKmsScreen::restoreMode()
                        &m_output.connector_id, 1,
                        &m_output.saved_crtc->mode);
 
-        drmModeFreeCrtc(m_output.saved_crtc);
-        m_output.saved_crtc = Q_NULLPTR;
-
         m_output.mode_set = false;
     }
 }

src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.h

@@ -47,6 +47,7 @@ QT_BEGIN_NAMESPACE
 
 class QEglFSKmsDevice;
 class QEglFSKmsCursor;
+class QEglFSKmsInterruptHandler;
 
 struct QEglFSKmsOutput
 {
@@ -115,6 +116,8 @@ private:
     FrameBuffer *framebufferForBufferObject(gbm_bo *bo);
 
     static QMutex m_waitForFlipMutex;
+
+    QEglFSKmsInterruptHandler *m_interruptHandler;
 };
 
 QT_END_NAMESPACE