aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMartin Gräßlin <[email protected]>2017-05-03 21:17:25 +0200
committerMartin Gräßlin <[email protected]>2017-05-03 21:36:34 +0200
commit0bec9ad7337536e319c17c5684d97e1156399fdb (patch)
tree01b5ac172f957edf068967cbda02206cb9c46014
parentf5a43877a9ea6ddad9eaa8d7498c8ea518c29c81 (diff)
Improve the x11 timestamp handling
Summary: So far KWin only updated the x11 timestamp if the new timestamp is larger than the existing one. While this is a useful thing it creates problems when the 32 bit msec based time stamp wraps around which happens after running an X server for 49 days. After the timestamp wrapped around KWin would not update the timestamp any more and thus some calls might fail. Most prominent victims are keyboard and pointer grab which fails as the timestamp is either larger than the server timestamp or smaller than the last grab timestamp. Another problem related to timestamp handling is KWin getting broken by wrong timestamps sent by applications. A prominent example is clusterssh which used to send a timestamp as unix time which is larger than the x timestamp and thus our timestamp gets too large. This change addresses these problems by allowing to reset the timestamp. This is only used from updateXTime (which is normally invoked before we do things like grabKeyboard). Thus we make QX11Info::getTimestamp the ultimate trusted source for timestamps. BUG: 377901 BUG: 348569 FIXED-IN: 5.8.7 Test Plan: As I cannot wait 50 days: unit tests for the two conditions added. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D5704
-rw-r--r--autotests/CMakeLists.txt11
-rw-r--r--autotests/test_x11_timestamp_update.cpp123
-rw-r--r--main.h8
-rw-r--r--utils.cpp2
-rw-r--r--utils.h6
5 files changed, 144 insertions, 6 deletions
diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index 2974d96..d4fdc79 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -310,3 +310,14 @@ target_link_libraries(testScreenEdges
add_test(kwin_testScreenEdges testScreenEdges)
ecm_mark_as_test(testScreenEdges)
+
+########################################################
+# Test X11 TimestampUpdate
+########################################################
+add_executable(testX11TimestampUpdate test_x11_timestamp_update.cpp)
+target_link_libraries(testX11TimestampUpdate
+ Qt5::Test
+ kwin
+)
+add_test(kwin-testX11TimestampUpdate testX11TimestampUpdate)
+ecm_mark_as_test(testX11TimestampUpdate)
diff --git a/autotests/test_x11_timestamp_update.cpp b/autotests/test_x11_timestamp_update.cpp
new file mode 100644
index 0000000..50ba151
--- /dev/null
+++ b/autotests/test_x11_timestamp_update.cpp
@@ -0,0 +1,123 @@
+/********************************************************************
+KWin - the KDE window manager
+This file is part of the KDE project.
+
+Copyright (C) 2017 Martin Gräßlin <[email protected]>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program. If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+#include <QTest>
+#include <QX11Info>
+
+#include "main.h"
+#include "utils.h"
+
+namespace KWin
+{
+
+class X11TestApplication : public Application
+{
+ Q_OBJECT
+public:
+ X11TestApplication(int &argc, char **argv);
+ virtual ~X11TestApplication();
+
+protected:
+ void performStartup() override;
+
+};
+
+X11TestApplication::X11TestApplication(int &argc, char **argv)
+ : Application(OperationModeX11, argc, argv)
+{
+ setX11Connection(QX11Info::connection());
+ setX11RootWindow(QX11Info::appRootWindow());
+}
+
+X11TestApplication::~X11TestApplication()
+{
+}
+
+void X11TestApplication::performStartup()
+{
+}
+
+}
+
+class X11TimestampUpdateTest : public QObject
+{
+ Q_OBJECT
+private Q_SLOTS:
+ void testGrabAfterServerTime();
+ void testBeforeLastGrabTime();
+};
+
+void X11TimestampUpdateTest::testGrabAfterServerTime()
+{
+ // this test tries to grab the X keyboard with a timestamp in future
+ // that should fail, but after updating the X11 timestamp, it should
+ // work again
+ KWin::updateXTime();
+ QCOMPARE(KWin::grabXKeyboard(), true);
+ KWin::ungrabXKeyboard();
+
+ // now let's change the timestamp
+ KWin::kwinApp()->setX11Time(KWin::xTime() + 5 * 60 * 1000);
+
+ // now grab keyboard should fail
+ QCOMPARE(KWin::grabXKeyboard(), false);
+
+ // let's update timestamp, now it should work again
+ KWin::updateXTime();
+ QCOMPARE(KWin::grabXKeyboard(), true);
+ KWin::ungrabXKeyboard();
+}
+
+void X11TimestampUpdateTest::testBeforeLastGrabTime()
+{
+ // this test tries to grab the X keyboard with a timestamp before the
+ // last grab time on the server. That should fail, but after updating the X11
+ // timestamp it should work again
+
+ // first set the grab timestamp
+ KWin::updateXTime();
+ QCOMPARE(KWin::grabXKeyboard(), true);
+ KWin::ungrabXKeyboard();
+
+ // now go to past
+ const auto timestamp = KWin::xTime();
+ KWin::kwinApp()->setX11Time(KWin::xTime() - 5 * 60 * 1000, KWin::Application::TimestampUpdate::Always);
+ QCOMPARE(KWin::xTime(), timestamp - 5 * 60 * 1000);
+
+ // now grab keyboard should fail
+ QCOMPARE(KWin::grabXKeyboard(), false);
+
+ // let's update timestamp, now it should work again
+ KWin::updateXTime();
+ QVERIFY(KWin::xTime() >= timestamp);
+ QCOMPARE(KWin::grabXKeyboard(), true);
+ KWin::ungrabXKeyboard();
+}
+
+int main(int argc, char *argv[])
+{
+ setenv("QT_QPA_PLATFORM", "xcb", true);
+ KWin::X11TestApplication app(argc, argv);
+ app.setAttribute(Qt::AA_Use96Dpi, true);
+ X11TimestampUpdateTest tc;
+ return QTest::qExec(&tc, argc, argv);
+}
+
+#include "test_x11_timestamp_update.moc"
diff --git a/main.h b/main.h
index a7dd47d..6706c0f 100644
--- a/main.h
+++ b/main.h
@@ -104,8 +104,12 @@ public:
xcb_timestamp_t x11Time() const {
return m_x11Time;
}
- void setX11Time(xcb_timestamp_t timestamp) {
- if (timestamp > m_x11Time) {
+ enum class TimestampUpdate {
+ OnlyIfLarger,
+ Always
+ };
+ void setX11Time(xcb_timestamp_t timestamp, TimestampUpdate force = TimestampUpdate::OnlyIfLarger) {
+ if (timestamp > m_x11Time || force == TimestampUpdate::Always) {
m_x11Time = timestamp;
}
}
diff --git a/utils.cpp b/utils.cpp
index ad45406..8d3f2ff 100644
--- a/utils.cpp
+++ b/utils.cpp
@@ -84,7 +84,7 @@ void updateXTime()
// NOTE: QX11Info::getTimestamp does not yet search the event queue as the old
// solution did. This means there might be regressions currently. See the
// documentation above on how it should be done properly.
- kwinApp()->setX11Time(QX11Info::getTimestamp());
+ kwinApp()->setX11Time(QX11Info::getTimestamp(), Application::TimestampUpdate::Always);
}
static int server_grab_count = 0;
diff --git a/utils.h b/utils.h
index e9fa912..06980fd 100644
--- a/utils.h
+++ b/utils.h
@@ -143,12 +143,12 @@ MaximizeMode operator^(MaximizeMode m1, MaximizeMode m2)
template <typename T> using ScopedCPointer = QScopedPointer<T, QScopedPointerPodDeleter>;
-void updateXTime();
+void KWIN_EXPORT updateXTime();
void grabXServer();
void ungrabXServer();
bool grabbedXServer();
-bool grabXKeyboard(xcb_window_t w = rootWindow());
-void ungrabXKeyboard();
+bool KWIN_EXPORT grabXKeyboard(xcb_window_t w = rootWindow());
+void KWIN_EXPORT ungrabXKeyboard();
/**
* Small helper class which performs @link grabXServer in the ctor and