xcb: utilize thread-safety of QAbstractEventDispatcher::wakeUp

QAbstractEventDispatcher::wakeUp is a thread-safe method, using
a queued connection to invoke it is wasteful. This type of connection
involves allocating temporary QMetaCallEvent on a heap and locking of
destination thread's post-event queue. In most use cases this is ok,
and really convenient when target method is not thread-safe. But in
this case the existing solution was suboptimal, especially because
the events we are reading can be high frequency.

The solution that we use here is lock-free. There can be only one
time when it might need to wait for the lock, which is upon exiting
the application. If we have entered the critical section in
QXcbEventReader::run(), then the registered post routine (qAddPostRoutine)
will block the QCoreApplication's dtor (this is where dispatcher is
set to 0) until we exit the critical section. We also know when not
to enter the critical section, in case dtor is already running.

With this approach we might need to compete for the lock at most
once, instead of whole application lifetime, which was the case
with the existing code.

Change-Id: If6737329c972347b0050d67658e28dbaa6f552e8
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

3 files changed, 22 insertions, 14 deletions

diff --git a/src/plugins/platforms/xcb/qxcbeventqueue.cpp b/src/plugins/platforms/xcb/qxcbeventqueue.cpp
index 6d8b001faa..5bd4e7a68a 100644
--- a/src/plugins/platforms/xcb/qxcbeventqueue.cpp
+++ b/src/plugins/platforms/xcb/qxcbeventqueue.cpp
@@ -40,11 +40,16 @@
 #include "qxcbconnection.h"
 
 #include <QtCore/QObject>
+#include <QtCore/QCoreApplication>
 #include <QtCore/QAbstractEventDispatcher>
+#include <QtCore/QMutex>
 #include <QtCore/QDebug>
 
 QT_BEGIN_NAMESPACE
 
+static QBasicMutex qAppExiting;
+static bool dispatcherOwnerDestructing = false;
+
 /*!
     \class QXcbEventQueue
     \internal
@@ -78,6 +83,15 @@ QXcbEventQueue::QXcbEventQueue(QXcbConnection *connection)
 {
     connect(this, &QXcbEventQueue::finished, m_connection, &QXcbConnection::processXcbEvents);
 
+    // When running test cases in auto tests, static variables are preserved
+    // between test function runs, even if Q*Application object is destroyed.
+    // Reset to default value to account for this.
+    dispatcherOwnerDestructing = false;
+    qAddPostRoutine([]() {
+        QMutexLocker locker(&qAppExiting);
+        dispatcherOwnerDestructing = true;
+    });
+
     // Lets init the list with one node, so we don't have to check for
     // this special case in various places.
     m_head = m_flushedTail = qXcbEventNodeFactory(nullptr);
@@ -102,11 +116,6 @@ QXcbEventQueue::~QXcbEventQueue()
     qCDebug(lcQpaEventReader) << "nodes on heap:" << m_nodesOnHeap;
 }
 
-void QXcbEventQueue::registerEventDispatcher(QAbstractEventDispatcher *dispatcher)
-{
-    connect(this, &QXcbEventQueue::eventsPending, dispatcher, &QAbstractEventDispatcher::wakeUp, Qt::QueuedConnection);
-}
-
 xcb_generic_event_t *QXcbEventQueue::takeFirst()
 {
     if (isEmpty())
@@ -191,7 +200,13 @@ void QXcbEventQueue::run()
         while (!m_closeConnectionDetected && (event = xcb_poll_for_queued_event(connection)))
             enqueueEvent(event);
 
-        emit eventsPending();
+        QMutexLocker locker(&qAppExiting);
+        if (!dispatcherOwnerDestructing) {
+            // This thread can run before a dispatcher has been created,
+            // so check if it is ready.
+            if (QCoreApplication::eventDispatcher())
+                QCoreApplication::eventDispatcher()->wakeUp();
+        }
     }
 }
 
diff --git a/src/plugins/platforms/xcb/qxcbeventqueue.h b/src/plugins/platforms/xcb/qxcbeventqueue.h
index 5b0d8bf3d6..bec2a6a201 100644
--- a/src/plugins/platforms/xcb/qxcbeventqueue.h
+++ b/src/plugins/platforms/xcb/qxcbeventqueue.h
@@ -80,8 +80,6 @@ public:
 
     void run() override;
 
-    void registerEventDispatcher(QAbstractEventDispatcher *dispatcher);
-
     bool isEmpty() const { return m_head == m_flushedTail && !m_head->event; }
     xcb_generic_event_t *takeFirst();
     void flushBufferedEvents();
@@ -101,8 +99,6 @@ public:
     using PeekerCallback = bool (*)(xcb_generic_event_t *event, void *peekerData);
     bool peekEventQueue(PeekerCallback peeker, void *peekerData = nullptr,
                         PeekOptions option = PeekDefault, qint32 peekerId = -1);
-signals:
-    void eventsPending();
 
 private:
     QXcbEventNode *qXcbEventNodeFactory(xcb_generic_event_t *event);
diff --git a/src/plugins/platforms/xcb/qxcbintegration.cpp b/src/plugins/platforms/xcb/qxcbintegration.cpp
index 8f6d35dc5b..d030b22fc8 100644
--- a/src/plugins/platforms/xcb/qxcbintegration.cpp
+++ b/src/plugins/platforms/xcb/qxcbintegration.cpp
@@ -343,10 +343,7 @@ bool QXcbIntegration::hasCapability(QPlatformIntegration::Capability cap) const
 
 QAbstractEventDispatcher *QXcbIntegration::createEventDispatcher() const
 {
-    QAbstractEventDispatcher *dispatcher = QXcbEventDispatcher::createEventDispatcher(defaultConnection());
-    for (int i = 0; i < m_connections.size(); i++)
-        m_connections[i]->eventQueue()->registerEventDispatcher(dispatcher);
-    return dispatcher;
+    return QXcbEventDispatcher::createEventDispatcher(defaultConnection());
 }
 
 void QXcbIntegration::initialize()