From 0c250d30bc6bc46b3db4a46469eca5622294ec6d Mon Sep 17 00:00:00 2001 From: Jan Grulich Date: Fri, 14 Sep 2018 08:36:59 +0200 Subject: [PATCH] Fix remote access buffer handling when output not bound --- kf5-kwayland.spec | 7 +- ...uffer-handling-when-output-not-bound.patch | 491 ++++++++++++++++++ 2 files changed, 497 insertions(+), 1 deletion(-) create mode 100644 kwayland-fix-remote-access-buffer-handling-when-output-not-bound.patch diff --git a/kf5-kwayland.spec b/kf5-kwayland.spec index 6c8b197..624ff05 100644 --- a/kf5-kwayland.spec +++ b/kf5-kwayland.spec @@ -14,7 +14,7 @@ Name: kf5-%{framework} Version: 5.50.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: KDE Frameworks 5 library that wraps Client and Server Wayland libraries License: GPLv2+ @@ -28,6 +28,8 @@ URL: https://cgit.kde.org/%{framework}.git %endif Source0: http://download.kde.org/%{stable}/frameworks/%{versiondir}/%{framework}-%{version}.tar.xz +Patch0: kwayland-fix-remote-access-buffer-handling-when-output-not-bound.patch + BuildRequires: extra-cmake-modules >= %{version} BuildRequires: kf5-rpm-macros >= %{version} BuildRequires: libwayland-client-devel >= %{wayland_min_version} @@ -113,6 +115,9 @@ make test ARGS="--output-on-failure --timeout 20" -C %{_target_platform} ||: %changelog +* Fri Sep 14 2018 Jan Grulich - 5.50.0-2 +- Fix remote access buffer handling when output not bound + * Tue Sep 04 2018 Rex Dieter - 5.50.0-1 - 5.50.0 diff --git a/kwayland-fix-remote-access-buffer-handling-when-output-not-bound.patch b/kwayland-fix-remote-access-buffer-handling-when-output-not-bound.patch new file mode 100644 index 0000000..f71be5c --- /dev/null +++ b/kwayland-fix-remote-access-buffer-handling-when-output-not-bound.patch @@ -0,0 +1,491 @@ +ndiff --git a/autotests/client/test_remote_access.cpp b/autotests/client/test_remote_access.cpp +--- a/autotests/client/test_remote_access.cpp ++++ b/autotests/client/test_remote_access.cpp +@@ -1,5 +1,6 @@ + /******************************************************************** + Copyright 2016 Oleg Chernovskiy ++Copyright 2018 Roman Gilg + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public +@@ -48,22 +49,108 @@ + + void testSendReleaseSingle(); + void testSendReleaseMultiple(); ++ void testSendReleaseCrossScreen(); + void testSendClientGone(); + void testSendReceiveClientGone(); + + private: + Display *m_display = nullptr; +- OutputInterface *m_outputInterface = nullptr; ++ OutputInterface *m_outputInterface[2] = {nullptr}; + RemoteAccessManagerInterface *m_remoteAccessInterface = nullptr; +- ConnectionThread *m_connection = nullptr; +- QThread *m_thread = nullptr; +- EventQueue *m_queue = nullptr; +- Registry *m_registry = nullptr; +- Output *m_output = nullptr; ++}; ++ ++class MockupClient : public QObject ++{ ++ Q_OBJECT ++public: ++ MockupClient(QObject *parent = nullptr); ++ ~MockupClient(); ++ ++ void bindOutput(int index); ++ ++ ConnectionThread *connection = nullptr; ++ QThread *thread = nullptr; ++ EventQueue *queue = nullptr; ++ Registry *registry = nullptr; ++ RemoteAccessManager *remoteAccess = nullptr; ++ Output *outputs[2] = {nullptr}; + }; + + static const QString s_socketName = QStringLiteral("kwayland-test-remote-access-0"); + ++MockupClient::MockupClient(QObject *parent) ++ : QObject(parent) ++{ ++ // setup connection ++ connection = new KWayland::Client::ConnectionThread; ++ QSignalSpy connectedSpy(connection, &ConnectionThread::connected); ++ QVERIFY(connectedSpy.isValid()); ++ connection->setSocketName(s_socketName); ++ ++ thread = new QThread(this); ++ connection->moveToThread(thread); ++ thread->start(); ++ ++ connection->initConnection(); ++ QVERIFY(connectedSpy.wait()); ++ ++ queue = new EventQueue(this); ++ queue->setup(connection); ++ ++ registry = new Registry(this); ++ QSignalSpy interfacesAnnouncedSpy(registry, &Registry::interfacesAnnounced); ++ QVERIFY(interfacesAnnouncedSpy.isValid()); ++ registry->setEventQueue(queue); ++ registry->create(connection); ++ QVERIFY(registry->isValid()); ++ registry->setup(); ++ QVERIFY(interfacesAnnouncedSpy.wait()); ++ ++ remoteAccess = registry->createRemoteAccessManager( ++ registry->interface(Registry::Interface::RemoteAccessManager).name, ++ registry->interface(Registry::Interface::RemoteAccessManager).version, ++ this); ++ QVERIFY(remoteAccess->isValid()); ++ connection->flush(); ++} ++ ++MockupClient::~MockupClient() ++{ ++#define CLEANUP(variable) \ ++ if (variable) { \ ++ delete variable; \ ++ variable = nullptr; \ ++ } ++ CLEANUP(outputs[0]) ++ CLEANUP(outputs[1]) ++ CLEANUP(remoteAccess) ++ CLEANUP(queue) ++ CLEANUP(registry) ++ if (thread) { ++ if (connection) { ++ connection->flush(); ++ connection->deleteLater(); ++ connection = nullptr; ++ } ++ ++ thread->quit(); ++ thread->wait(); ++ delete thread; ++ thread = nullptr; ++ } ++#undef CLEANUP ++} ++ ++void MockupClient::bindOutput(int index) ++{ ++ // client-bound output ++ outputs[index] = registry->createOutput(registry->interfaces(Registry::Interface::Output)[index].name, ++ registry->interfaces(Registry::Interface::Output)[index].version, ++ this); ++ QVERIFY(outputs[index]->isValid()); ++ connection->flush(); ++} ++ + void RemoteAccessTest::init() + { + qRegisterMetaType(); +@@ -76,42 +163,18 @@ + m_display->start(); + QVERIFY(m_display->isRunning()); + m_display->createShm(); +- m_outputInterface = m_display->createOutput(); +- m_outputInterface->create(); ++ ++ auto initOutputIface = [this](int i) { ++ m_outputInterface[i] = m_display->createOutput(); ++ m_outputInterface[i]->create(); ++ }; ++ initOutputIface(0); ++ initOutputIface(1); ++ + m_remoteAccessInterface = m_display->createRemoteAccessManager(); + m_remoteAccessInterface->create(); + QSignalSpy bufferReleasedSpy(m_remoteAccessInterface, &RemoteAccessManagerInterface::bufferReleased); + QVERIFY(bufferReleasedSpy.isValid()); +- +- // setup connection +- m_connection = new KWayland::Client::ConnectionThread; +- QSignalSpy connectedSpy(m_connection, &ConnectionThread::connected); +- QVERIFY(connectedSpy.isValid()); +- m_connection->setSocketName(s_socketName); +- +- m_thread = new QThread(this); +- m_connection->moveToThread(m_thread); +- m_thread->start(); +- +- m_connection->initConnection(); +- QVERIFY(connectedSpy.wait()); +- +- m_queue = new EventQueue(this); +- m_queue->setup(m_connection); +- +- m_registry = new Registry(this); +- QSignalSpy interfacesAnnouncedSpy(m_registry, &Registry::interfacesAnnounced); +- QVERIFY(interfacesAnnouncedSpy.isValid()); +- m_registry->setEventQueue(m_queue); +- m_registry->create(m_connection); +- QVERIFY(m_registry->isValid()); +- m_registry->setup(); +- QVERIFY(interfacesAnnouncedSpy.wait()); +- +- // client-bound output +- m_output = m_registry->createOutput(m_registry->interface(Registry::Interface::Output).name, +- m_registry->interface(Registry::Interface::Output).version, +- this); + } + + void RemoteAccessTest::cleanup() +@@ -121,22 +184,8 @@ + delete variable; \ + variable = nullptr; \ + } +- CLEANUP(m_output) +- CLEANUP(m_queue) +- CLEANUP(m_registry) +- if (m_thread) { +- if (m_connection) { +- m_connection->flush(); +- m_connection->deleteLater(); +- m_connection = nullptr; +- } +- +- m_thread->quit(); +- m_thread->wait(); +- delete m_thread; +- m_thread = nullptr; +- } +- ++ CLEANUP(m_outputInterface[0]) ++ CLEANUP(m_outputInterface[1]) + CLEANUP(m_remoteAccessInterface) + CLEANUP(m_display) + #undef CLEANUP +@@ -148,16 +197,13 @@ + + // setup + QVERIFY(!m_remoteAccessInterface->isBound()); +- auto client = m_registry->createRemoteAccessManager( +- m_registry->interface(Registry::Interface::RemoteAccessManager).name, +- m_registry->interface(Registry::Interface::RemoteAccessManager).version, +- this); +- QVERIFY(client->isValid()); +- m_connection->flush(); ++ auto *client = new MockupClient(this); ++ client->bindOutput(0); ++ + m_display->dispatchEvents(); + + QVERIFY(m_remoteAccessInterface->isBound()); // we have one client now +- QSignalSpy bufferReadySpy(client, &RemoteAccessManager::bufferReady); ++ QSignalSpy bufferReadySpy(client->remoteAccess, &RemoteAccessManager::bufferReady); + QVERIFY(bufferReadySpy.isValid()); + + BufferHandle *buf = new BufferHandle(); +@@ -168,7 +214,7 @@ + buf->setSize(50, 50); + buf->setFormat(100500); + buf->setStride(7800); +- m_remoteAccessInterface->sendBufferReady(m_outputInterface, buf); ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[0], buf); + + // receive buffer + QVERIFY(bufferReadySpy.wait()); +@@ -193,7 +239,6 @@ + // cleanup + delete buf; + delete client; +- m_connection->flush(); + m_display->dispatchEvents(); + QVERIFY(!m_remoteAccessInterface->isBound()); + } +@@ -204,23 +249,16 @@ + + // setup + QVERIFY(!m_remoteAccessInterface->isBound()); +- auto client1 = m_registry->createRemoteAccessManager( +- m_registry->interface(Registry::Interface::RemoteAccessManager).name, +- m_registry->interface(Registry::Interface::RemoteAccessManager).version, +- this); +- QVERIFY(client1->isValid()); +- auto client2 = m_registry->createRemoteAccessManager( +- m_registry->interface(Registry::Interface::RemoteAccessManager).name, +- m_registry->interface(Registry::Interface::RemoteAccessManager).version, +- this); +- QVERIFY(client2->isValid()); +- m_connection->flush(); ++ auto *client1 = new MockupClient(this); ++ auto *client2 = new MockupClient(this); ++ client1->bindOutput(0); ++ client2->bindOutput(0); + m_display->dispatchEvents(); +- + QVERIFY(m_remoteAccessInterface->isBound()); // now we have 2 clients +- QSignalSpy bufferReadySpy1(client1, &RemoteAccessManager::bufferReady); ++ ++ QSignalSpy bufferReadySpy1(client1->remoteAccess, &RemoteAccessManager::bufferReady); + QVERIFY(bufferReadySpy1.isValid()); +- QSignalSpy bufferReadySpy2(client2, &RemoteAccessManager::bufferReady); ++ QSignalSpy bufferReadySpy2(client2->remoteAccess, &RemoteAccessManager::bufferReady); + QVERIFY(bufferReadySpy2.isValid()); + + BufferHandle *buf = new BufferHandle(); +@@ -231,10 +269,13 @@ + buf->setSize(50, 50); + buf->setFormat(100500); + buf->setStride(7800); +- m_remoteAccessInterface->sendBufferReady(m_outputInterface, buf); ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[0], buf); + + // wait for event loop + QVERIFY(bufferReadySpy1.wait()); ++ if (bufferReadySpy2.size() == 0) { ++ QVERIFY(bufferReadySpy2.wait()); ++ } + + // receive buffer at client 1 + QCOMPARE(bufferReadySpy1.size(), 1); +@@ -251,6 +292,9 @@ + // wait for event loop + QVERIFY(paramsObtainedSpy1.wait()); + QCOMPARE(paramsObtainedSpy1.size(), 1); ++ if (paramsObtainedSpy2.size() == 0) { ++ QVERIFY(paramsObtainedSpy2.wait()); ++ } + QCOMPARE(paramsObtainedSpy2.size(), 1); + + // release +@@ -266,25 +310,106 @@ + delete buf; + delete client1; + delete client2; +- m_connection->flush(); ++ m_display->dispatchEvents(); ++ QVERIFY(!m_remoteAccessInterface->isBound()); ++} ++ ++void RemoteAccessTest::testSendReleaseCrossScreen() ++{ ++ // this test verifies that multiple buffers for multiple screens are sent to ++ // multiple clients and returned back ++ ++ // setup ++ QVERIFY(!m_remoteAccessInterface->isBound()); ++ auto *client1 = new MockupClient(this); ++ auto *client2 = new MockupClient(this); ++ client1->bindOutput(1); ++ client2->bindOutput(0); ++ m_display->dispatchEvents(); ++ QVERIFY(m_remoteAccessInterface->isBound()); // now we have 2 clients ++ ++ QSignalSpy bufferReadySpy1(client1->remoteAccess, &RemoteAccessManager::bufferReady); ++ QVERIFY(bufferReadySpy1.isValid()); ++ QSignalSpy bufferReadySpy2(client2->remoteAccess, &RemoteAccessManager::bufferReady); ++ QVERIFY(bufferReadySpy2.isValid()); ++ ++ BufferHandle *buf1 = new BufferHandle(); ++ QTemporaryFile *tmpFile1 = new QTemporaryFile(this); ++ tmpFile1->open(); ++ ++ BufferHandle *buf2 = new BufferHandle(); ++ QTemporaryFile *tmpFile2 = new QTemporaryFile(this); ++ tmpFile2->open(); ++ ++ buf1->setFd(tmpFile1->handle()); ++ buf1->setSize(50, 50); ++ buf1->setFormat(100500); ++ buf1->setStride(7800); ++ ++ buf2->setFd(tmpFile2->handle()); ++ buf2->setSize(100, 100); ++ buf2->setFormat(100500); ++ buf2->setStride(7800); ++ ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[0], buf1); ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[1], buf2); ++ ++ // wait for event loop ++ QVERIFY(bufferReadySpy1.wait()); ++ if (bufferReadySpy2.size() == 0) { ++ QVERIFY(bufferReadySpy2.wait()); ++ } ++ ++ // receive buffer at client 1 ++ QCOMPARE(bufferReadySpy1.size(), 1); ++ auto rbuf1 = bufferReadySpy1.takeFirst()[1].value(); ++ QSignalSpy paramsObtainedSpy1(rbuf1, &RemoteBuffer::parametersObtained); ++ QVERIFY(paramsObtainedSpy1.isValid()); ++ ++ // receive buffer at client 2 ++ QCOMPARE(bufferReadySpy2.size(), 1); ++ auto rbuf2 = bufferReadySpy2.takeFirst()[1].value(); ++ QSignalSpy paramsObtainedSpy2(rbuf2, &RemoteBuffer::parametersObtained); ++ QVERIFY(paramsObtainedSpy2.isValid()); ++ ++ // wait for event loop ++ QVERIFY(paramsObtainedSpy1.wait()); ++ if (paramsObtainedSpy2.size() == 0) { ++ QVERIFY(paramsObtainedSpy2.wait()); ++ } ++ QCOMPARE(paramsObtainedSpy1.size(), 1); ++ QCOMPARE(paramsObtainedSpy2.size(), 1); ++ ++ // release ++ QSignalSpy bufferReleasedSpy(m_remoteAccessInterface, &RemoteAccessManagerInterface::bufferReleased); ++ QVERIFY(bufferReleasedSpy.isValid()); ++ delete rbuf1; ++ QVERIFY(bufferReleasedSpy.wait()); ++ ++ delete rbuf2; ++ QVERIFY(bufferReleasedSpy.wait()); ++ ++ QCOMPARE(bufferReleasedSpy.size(), 2); ++ ++ // cleanup ++ delete buf1; ++ delete buf2; ++ delete client1; ++ delete client2; + m_display->dispatchEvents(); + QVERIFY(!m_remoteAccessInterface->isBound()); + } + + void RemoteAccessTest::testSendClientGone() + { + // this test verifies that when buffer is sent and client is gone, server will release buffer correctly + QVERIFY(!m_remoteAccessInterface->isBound()); +- auto client = m_registry->createRemoteAccessManager( +- m_registry->interface(Registry::Interface::RemoteAccessManager).name, +- m_registry->interface(Registry::Interface::RemoteAccessManager).version, +- this); +- QVERIFY(client->isValid()); +- m_connection->flush(); ++ auto *client = new MockupClient(this); ++ client->bindOutput(0); + m_display->dispatchEvents(); + + QVERIFY(m_remoteAccessInterface->isBound()); // we have one client now +- QSignalSpy bufferReadySpy(client, &RemoteAccessManager::bufferReady); ++ QSignalSpy bufferReadySpy(client->remoteAccess, &RemoteAccessManager::bufferReady); + QVERIFY(bufferReadySpy.isValid()); + + BufferHandle *buf = new BufferHandle(); +@@ -295,7 +420,7 @@ + buf->setSize(50, 50); + buf->setFormat(100500); + buf->setStride(7800); +- m_remoteAccessInterface->sendBufferReady(m_outputInterface, buf); ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[0], buf); + + // release forcefully + QSignalSpy bufferReleasedSpy(m_remoteAccessInterface, &RemoteAccessManagerInterface::bufferReleased); +@@ -305,7 +430,6 @@ + + // cleanup + delete buf; +- m_connection->flush(); + m_display->dispatchEvents(); + QVERIFY(!m_remoteAccessInterface->isBound()); + } +@@ -315,16 +439,12 @@ + // this test verifies that when buffer is sent, received and client is gone, + // both client and server will release buffer correctly + QVERIFY(!m_remoteAccessInterface->isBound()); +- auto client = m_registry->createRemoteAccessManager( +- m_registry->interface(Registry::Interface::RemoteAccessManager).name, +- m_registry->interface(Registry::Interface::RemoteAccessManager).version, +- this); +- QVERIFY(client->isValid()); +- m_connection->flush(); ++ auto *client = new MockupClient(this); ++ client->bindOutput(0); + m_display->dispatchEvents(); + + QVERIFY(m_remoteAccessInterface->isBound()); // we have one client now +- QSignalSpy bufferReadySpy(client, &RemoteAccessManager::bufferReady); ++ QSignalSpy bufferReadySpy(client->remoteAccess, &RemoteAccessManager::bufferReady); + QVERIFY(bufferReadySpy.isValid()); + + BufferHandle *buf = new BufferHandle(); +@@ -335,7 +455,7 @@ + buf->setSize(50, 50); + buf->setFormat(100500); + buf->setStride(7800); +- m_remoteAccessInterface->sendBufferReady(m_outputInterface, buf); ++ m_remoteAccessInterface->sendBufferReady(m_outputInterface[0], buf); + + // receive buffer + QVERIFY(bufferReadySpy.wait()); +@@ -359,11 +479,9 @@ + + // cleanup + delete buf; +- m_connection->flush(); + m_display->dispatchEvents(); + QVERIFY(!m_remoteAccessInterface->isBound()); + } + +- + QTEST_GUILESS_MAIN(RemoteAccessTest) + #include "test_remote_access.moc" +diff --git a/src/server/remote_access_interface.cpp b/src/server/remote_access_interface.cpp +--- a/src/server/remote_access_interface.cpp ++++ b/src/server/remote_access_interface.cpp +@@ -204,13 +204,18 @@ + + // clients don't necessarily bind outputs + if (boundScreens.isEmpty()) { +- return; ++ continue; + } + + // no reason for client to bind wl_output multiple times, send only to first one + org_kde_kwin_remote_access_manager_send_buffer_ready(res, buf->fd(), boundScreens[0]); + holder.counter++; + } ++ if (holder.counter == 0) { ++ // buffer was not requested by any client ++ emit q->bufferReleased(buf); ++ return; ++ } + // store buffer locally, clients will ask it later + sentBuffers[buf->fd()] = holder; + }