Revamp signal handling in eglfs/linuxfb

Go back to the pipe-based signal handling. signalfd() introduces more harm than good
and is a regression for applications that install their own signal handlers.

Simplify the somewhat overcomplicated suspend (Ctrl+Z) logic too. There is no need for
requiring a callback. Just enable/disable the keyboard and cursor on suspend and resume
and emit the signals. Backends (like kms) may then perform additional steps, if they
choose to do so.

Task-number: QTBUG-48384
Change-Id: Ifd52de89c59915a2e0be6bf5ebc6f2ff1728eb50
Reviewed-by: Louai Al-Khanji <louai.al-khanji@theqtcompany.com>

3 files changed, 62 insertions, 70 deletions

diff --git a/src/platformsupport/fbconvenience/qfbvthandler.cpp b/src/platformsupport/fbconvenience/qfbvthandler.cpp
index c46e470c34..5e062b3f0a 100644
--- a/src/platformsupport/fbconvenience/qfbvthandler.cpp
+++ b/src/platformsupport/fbconvenience/qfbvthandler.cpp
@@ -79,87 +79,79 @@ static void setTTYCursor(bool enable)
 }
 #endif
 
+#ifdef VTH_ENABLED
+static QFbVtHandler *vth;
+
+void QFbVtHandler::signalHandler(int sigNo)
+{
+    char a = sigNo;
+    QT_WRITE(vth->m_sigFd[0], &a, sizeof(a));
+}
+#endif
+
 QFbVtHandler::QFbVtHandler(QObject *parent)
     : QObject(parent),
       m_tty(-1),
-      m_signalFd(-1),
       m_signalNotifier(0)
 {
 #ifdef VTH_ENABLED
-    setTTYCursor(false);
-
-    if (isatty(0)) {
+    if (isatty(0))
         m_tty = 0;
-        ioctl(m_tty, KDGKBMODE, &m_oldKbdMode);
 
-        if (!qEnvironmentVariableIntValue("QT_QPA_ENABLE_TERMINAL_KEYBOARD")) {
-            // Disable the tty keyboard.
-            ioctl(m_tty, KDSKBMUTE, 1);
-            ioctl(m_tty, KDSKBMODE, KBD_OFF_MODE);
-        }
+    if (::socketpair(AF_UNIX, SOCK_STREAM, 0, m_sigFd)) {
+        qErrnoWarning(errno, "QFbVtHandler: socketpair() failed");
+        return;
     }
 
-    // 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);
-    }
+    vth = this;
+    setTTYCursor(false);
+    setKeyboardEnabled(false);
+
+    m_signalNotifier = new QSocketNotifier(m_sigFd[1], QSocketNotifier::Read, this);
+    connect(m_signalNotifier, &QSocketNotifier::activated, this, &QFbVtHandler::handleSignal);
+
+    struct sigaction sa;
+    sa.sa_flags = 0;
+    sa.sa_handler = signalHandler;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGINT, &sa, 0); // Ctrl+C
+    sigaction(SIGTSTP, &sa, 0); // Ctrl+Z
+    sigaction(SIGCONT, &sa, 0);
+    sigaction(SIGTERM, &sa, 0); // default signal used by kill
 #endif
 }
 
 QFbVtHandler::~QFbVtHandler()
 {
 #ifdef VTH_ENABLED
-    restoreKeyboard();
+    setKeyboardEnabled(true);
     setTTYCursor(true);
 
-    if (m_signalFd != -1)
-        close(m_signalFd);
+    if (m_signalNotifier) {
+        close(m_sigFd[0]);
+        close(m_sigFd[1]);
+    }
 #endif
 }
 
-void QFbVtHandler::restoreKeyboard()
+void QFbVtHandler::setKeyboardEnabled(bool enable)
 {
 #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);
+    if (enable) {
+        ::ioctl(m_tty, KDSKBMUTE, 0);
+        ::ioctl(m_tty, KDSKBMODE, m_oldKbdMode);
+    } else {
+        ::ioctl(m_tty, KDGKBMODE, &m_oldKbdMode);
+        if (!qEnvironmentVariableIntValue("QT_QPA_ENABLE_TERMINAL_KEYBOARD")) {
+            ::ioctl(m_tty, KDSKBMUTE, 1);
+            ::ioctl(m_tty, KDSKBMODE, KBD_OFF_MODE);
+        }
+    }
+#else
+    Q_UNUSED(enable);
 #endif
 }
 
@@ -168,17 +160,22 @@ 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) {
+    char sigNo;
+    if (QT_READ(m_sigFd[1], &sigNo, sizeof(sigNo)) == sizeof(sigNo)) {
+        switch (sigNo) {
         case SIGINT: // fallthrough
         case SIGTERM:
             handleInt();
             break;
         case SIGTSTP:
-            emit suspendRequested();
+            emit aboutToSuspend();
+            setKeyboardEnabled(true);
+            setTTYCursor(true);
+            ::kill(getpid(), SIGSTOP);
             break;
         case SIGCONT:
+            setTTYCursor(false);
+            setKeyboardEnabled(false);
             emit resumed();
             break;
         default:
@@ -194,7 +191,7 @@ void QFbVtHandler::handleInt()
 {
 #ifdef VTH_ENABLED
     emit interrupted();
-    restoreKeyboard();
+    setKeyboardEnabled(true);
     setTTYCursor(true);
     _exit(1);
 #endif
diff --git a/src/platformsupport/fbconvenience/qfbvthandler_p.h b/src/platformsupport/fbconvenience/qfbvthandler_p.h
index 3681def9f0..91eba248fe 100644
--- a/src/platformsupport/fbconvenience/qfbvthandler_p.h
+++ b/src/platformsupport/fbconvenience/qfbvthandler_p.h
@@ -59,23 +59,22 @@ public:
     QFbVtHandler(QObject *parent = 0);
     ~QFbVtHandler();
 
-    void suspend();
-
 signals:
     void interrupted();
-    void suspendRequested();
+    void aboutToSuspend();
     void resumed();
 
 private slots:
     void handleSignal();
 
 private:
-    void restoreKeyboard();
+    void setKeyboardEnabled(bool enable);
     void handleInt();
+    static void signalHandler(int sigNo);
 
     int m_tty;
     int m_oldKbdMode;
-    int m_signalFd;
+    int m_sigFd[2];
     QSocketNotifier *m_signalNotifier;
 };
 
diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.cpp b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.cpp
index d2a86c1e25..60586f98a7 100644
--- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.cpp
+++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsscreen.cpp
@@ -52,15 +52,11 @@ public:
     QEglFSKmsInterruptHandler(QEglFSKmsScreen *screen) : m_screen(screen) {
         m_vtHandler = static_cast<QEglFSIntegration *>(QGuiApplicationPrivate::platformIntegration())->vtHandler();
         connect(m_vtHandler, &QFbVtHandler::interrupted, this, &QEglFSKmsInterruptHandler::restoreVideoMode);
-        connect(m_vtHandler, &QFbVtHandler::suspendRequested, this, &QEglFSKmsInterruptHandler::handleSuspendRequest);
+        connect(m_vtHandler, &QFbVtHandler::aboutToSuspend, this, &QEglFSKmsInterruptHandler::restoreVideoMode);
     }
 
 public slots:
     void restoreVideoMode() { m_screen->restoreMode(); }
-    void handleSuspendRequest() {
-        m_screen->restoreMode();
-        m_vtHandler->suspend();
-    }
 
 private:
     QFbVtHandler *m_vtHandler;