Merge lp:~gerboland/unity-2d/keymonitor-shutdown-fixes into lp:unity-2d

[Gerry Boland]

Rewrite KeyMonitor XEvent handler to use QSocketNotifier instead of separate thread. This singleton will now be deconstructed properly on application quit. Also free X11 objects when they're unnecessary.

[Alberto Mardegan (mardy)]

I tested this, but I don't see an improvement on session logout time.

[Gerry Boland (gerboland)]

No unfortunately it doesn't help with logout-time, I suspect the communication between gnome-session and Launcher at logout is incomplete.

But without this patch QCoreApplication::quit() does not quit the Launcher as the QtConcurrent thread does not halt. So this is a step in right direction, as otherwise the process needs to be killed.

This also keeps code consistency with GEIS gesture handler.

[Olivier Tilloy (osomon)]

The code looks good and clean, and it fixes the unit test that was hanging.

I have a tiny remark style-wise, which you may safely ignore if you feel like it: the x11FileDescriptor variable is referred to only once, so you can remove it and invoke directly ConnectionNumber(m_display) when instantiating the QSocketNotifier. Other than that, it all looks perfect to me.

As to bug #859596 I can’t really tell whether that fixes it, but it should considering comment #8.

review: Approve

=== modified file 'libunity-2d-private/src/keymonitor.cpp'
--- libunity-2d-private/src/keymonitor.cpp	2011-09-23 12:45:33 +0000
+++ libunity-2d-private/src/keymonitor.cpp	2011-10-28 22:27:24 +0000
@@ -21,7 +21,7 @@
 #include "keymonitor.h"
 
 // Qt
-#include <QtConcurrentRun>
+#include <QSocketNotifier>
 #include <QDebug>
 
 // X11
@@ -38,19 +38,17 @@
 
 
 KeyMonitor::KeyMonitor(QObject* parent)
-: QObject(parent), m_stop(false)
+: QObject(parent)
 {
     if (this->registerEvents()) {
         getModifiers();
-        m_future = QtConcurrent::run(this, &KeyMonitor::run);
     }
 }
 
 KeyMonitor::~KeyMonitor()
 {
-    /* let the running thread know that it should stop */
-    m_stop = true;
     m_eventList.clear();
+    XCloseDisplay(m_display);
 }
 
 KeyMonitor* KeyMonitor::instance()
@@ -74,6 +72,8 @@
             m_modList.append(xmodmap->modifiermap[i]);
         }
     }
+
+    XFreeModifiermap(xmodmap);
 }
 
 bool KeyMonitor::registerEvents()
@@ -82,6 +82,7 @@
     Window window;
 
     XDeviceInfo *devices;
+    int x11FileDescriptor;
     int num_devices;
     int i, j;
 
@@ -116,8 +117,11 @@
                 }
             }
         }
+        XCloseDevice(m_display, device);
     }
 
+    XFreeDeviceList(devices);
+
     if (m_eventList.size() == 0) {
         UQ_WARNING << "No input devices found.";
         return false;
@@ -128,15 +132,21 @@
         return false;
     }
 
+    /* Dispatch XEvents when there is activity on the X11 file descriptor */
+    x11FileDescriptor = ConnectionNumber(m_display);
+    QSocketNotifier* socketNotifier = new QSocketNotifier(x11FileDescriptor, QSocketNotifier::Read, this);
+    connect(socketNotifier, SIGNAL(activated(int)), this, SLOT(x11EventDispatch()));
+
     return true;
 }
 
 
-void KeyMonitor::run()
+void KeyMonitor::x11EventDispatch()
 {
     XEvent event;
 
-    while(!m_stop && !XNextEvent(m_display, &event)) {
+    while (XPending(m_display) > 0) {
+        XNextEvent(m_display, &event);
         if (event.type == key_press_type) {
             XDeviceKeyEvent *keyEvent = (XDeviceKeyEvent *) &event;
             if (!m_modList.contains((KeyCode) keyEvent->keycode)) {

=== modified file 'libunity-2d-private/src/keymonitor.h'
--- libunity-2d-private/src/keymonitor.h	2011-09-19 19:06:54 +0000
+++ libunity-2d-private/src/keymonitor.h	2011-10-28 22:27:24 +0000
@@ -51,11 +51,12 @@
 
     void getModifiers();
     bool registerEvents();
-    void run();
-
+
+private Q_SLOTS:
+    void x11EventDispatch();
+
+private:
     Display *m_display;
-    QFuture<void> m_future;
-    bool m_stop;
     QVector<XEventClass> m_eventList;
     QVector<KeyCode> m_modList;
 };