Commit fd01e59a authored by Martin Flöser's avatar Martin Flöser
Browse files

[server] Improve the handling when clients disconnect

Summary:
So far the server component performed manual cleanup in some cases
when a client disconnects. But this is not needed: the Wayland library
calls the static unbind methods which do cleanup. If we cleanup ourselves
this can result in double deletes in the worst case, so let's only use
the Wayland functionality.

Adjusted:
* RegionInterface
* SurfaceInterface
* ShellSurfaceInterface (doesn't take a parent anymore)
* DpmsInterface
* QtSurfaceExtensionInterface
* KeyboardInterface
* PointerInterface
* TouchInterface
* DataOfferInterface
* PlasmaShellSurfaceInterface

For each adjusted case a test case is added to verify that the cleanup
works. Exceptions are DpmsInterface as the actual Resource is not exposed
at all in the Server component and DataOfferInterface as that is server
side created.

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D1640
parent 50279557
......@@ -45,6 +45,7 @@ private Q_SLOTS:
void testRole_data();
void testRole();
void testDisconnect();
private:
Display *m_display = nullptr;
......@@ -190,5 +191,43 @@ void TestPlasmaShell::testRole()
QTEST(sps->role(), "serverRole");
}
void TestPlasmaShell::testDisconnect()
{
// this test verifies that a disconnect cleans up
QSignalSpy plasmaSurfaceCreatedSpy(m_plasmaShellInterface, &PlasmaShellInterface::surfaceCreated);
QVERIFY(plasmaSurfaceCreatedSpy.isValid());
// create the surface
QScopedPointer<Surface> s(m_compositor->createSurface());
QScopedPointer<PlasmaShellSurface> ps(m_plasmaShell->createSurface(s.data()));
// and get them on the server
QVERIFY(plasmaSurfaceCreatedSpy.wait());
QCOMPARE(plasmaSurfaceCreatedSpy.count(), 1);
auto sps = plasmaSurfaceCreatedSpy.first().first().value<PlasmaShellSurfaceInterface*>();
QVERIFY(sps);
// disconnect
QSignalSpy clientDisconnectedSpy(sps->client(), &ClientConnection::disconnected);
QVERIFY(clientDisconnectedSpy.isValid());
QSignalSpy surfaceDestroyedSpy(sps, &QObject::destroyed);
QVERIFY(surfaceDestroyedSpy.isValid());
if (m_connection) {
m_connection->deleteLater();
m_connection = nullptr;
}
QVERIFY(clientDisconnectedSpy.wait());
QCOMPARE(clientDisconnectedSpy.count(), 1);
QCOMPARE(surfaceDestroyedSpy.count(), 0);
QVERIFY(surfaceDestroyedSpy.wait());
QCOMPARE(surfaceDestroyedSpy.count(), 1);
s->destroy();
ps->destroy();
m_plasmaShell->destroy();
m_compositor->destroy();
m_registry->destroy();
m_queue->destroy();
}
QTEST_GUILESS_MAIN(TestPlasmaShell)
#include "test_plasmashell.moc"
......@@ -44,6 +44,7 @@ private Q_SLOTS:
void testAdd();
void testRemove();
void testDestroy();
void testDisconnect();
private:
KWayland::Server::Display *m_display;
......@@ -281,5 +282,38 @@ void TestRegion::testDestroy()
region->destroy();
}
void TestRegion::testDisconnect()
{
// this test verifies that the server side correctly tears down the resources when the client disconnects
using namespace KWayland::Client;
using namespace KWayland::Server;
QScopedPointer<Region> r(m_compositor->createRegion());
QVERIFY(!r.isNull());
QVERIFY(r->isValid());
QSignalSpy regionCreatedSpy(m_compositorInterface, &CompositorInterface::regionCreated);
QVERIFY(regionCreatedSpy.isValid());
QVERIFY(regionCreatedSpy.wait());
auto serverRegion = regionCreatedSpy.first().first().value<RegionInterface*>();
// destroy client
QSignalSpy clientDisconnectedSpy(serverRegion->client(), &ClientConnection::disconnected);
QVERIFY(clientDisconnectedSpy.isValid());
QSignalSpy regionDestroyedSpy(serverRegion, &QObject::destroyed);
QVERIFY(regionDestroyedSpy.isValid());
if (m_connection) {
m_connection->deleteLater();
m_connection = nullptr;
}
QVERIFY(clientDisconnectedSpy.wait());
QCOMPARE(clientDisconnectedSpy.count(), 1);
QCOMPARE(regionDestroyedSpy.count(), 0);
QVERIFY(regionDestroyedSpy.wait());
QCOMPARE(regionDestroyedSpy.count(), 1);
r->destroy();
m_compositor->destroy();
m_queue->destroy();
}
QTEST_GUILESS_MAIN(TestRegion)
#include "test_wayland_region.moc"
......@@ -75,6 +75,7 @@ private Q_SLOTS:
void testDestroy();
void testSelection();
void testTouch();
void testDisconnect();
// TODO: add test for keymap
private:
......@@ -1535,5 +1536,77 @@ void TestWaylandSeat::testTouch()
QCOMPARE(touch->sequence().first()->position(), QPointF(0, 0));
}
void TestWaylandSeat::testDisconnect()
{
// this test verifies that disconnecting the client cleans up correctly
using namespace KWayland::Client;
using namespace KWayland::Server;
QSignalSpy keyboardCreatedSpy(m_seatInterface, &SeatInterface::keyboardCreated);
QVERIFY(keyboardCreatedSpy.isValid());
QSignalSpy pointerCreatedSpy(m_seatInterface, &SeatInterface::pointerCreated);
QVERIFY(pointerCreatedSpy.isValid());
QSignalSpy touchCreatedSpy(m_seatInterface, &SeatInterface::touchCreated);
QVERIFY(touchCreatedSpy.isValid());
// create the things we need
m_seatInterface->setHasKeyboard(true);
m_seatInterface->setHasPointer(true);
m_seatInterface->setHasTouch(true);
QSignalSpy touchSpy(m_seat, &Seat::hasTouchChanged);
QVERIFY(touchSpy.isValid());
QVERIFY(touchSpy.wait());
QScopedPointer<Keyboard> keyboard(m_seat->createKeyboard());
QVERIFY(!keyboard.isNull());
QVERIFY(keyboardCreatedSpy.wait());
auto serverKeyboard = keyboardCreatedSpy.first().first().value<KeyboardInterface*>();
QVERIFY(serverKeyboard);
QScopedPointer<Pointer> pointer(m_seat->createPointer());
QVERIFY(!pointer.isNull());
QVERIFY(pointerCreatedSpy.wait());
auto serverPointer = pointerCreatedSpy.first().first().value<PointerInterface*>();
QVERIFY(serverPointer);
QScopedPointer<Touch> touch(m_seat->createTouch());
QVERIFY(!touch.isNull());
QVERIFY(touchCreatedSpy.wait());
auto serverTouch = touchCreatedSpy.first().first().value<TouchInterface*>();
QVERIFY(serverTouch);
// setup destroys
QSignalSpy keyboardDestroyedSpy(serverKeyboard, &QObject::destroyed);
QVERIFY(keyboardDestroyedSpy.isValid());
QSignalSpy pointerDestroyedSpy(serverPointer, &QObject::destroyed);
QVERIFY(pointerDestroyedSpy.isValid());
QSignalSpy touchDestroyedSpy(serverTouch, &QObject::destroyed);
QVERIFY(touchDestroyedSpy.isValid());
QSignalSpy clientDisconnectedSpy(serverKeyboard->client(), &ClientConnection::disconnected);
QVERIFY(clientDisconnectedSpy.isValid());
if (m_connection) {
m_connection->deleteLater();
m_connection = nullptr;
}
QVERIFY(clientDisconnectedSpy.wait());
QCOMPARE(clientDisconnectedSpy.count(), 1);
QCOMPARE(keyboardDestroyedSpy.count(), 0);
QCOMPARE(pointerDestroyedSpy.count(), 0);
QCOMPARE(touchDestroyedSpy.count(), 0);
QVERIFY(keyboardDestroyedSpy.wait());
QCOMPARE(keyboardDestroyedSpy.count(), 1);
QCOMPARE(pointerDestroyedSpy.count(), 1);
QCOMPARE(touchDestroyedSpy.count(), 1);
keyboard->destroy();
pointer->destroy();
touch->destroy();
m_compositor->destroy();
m_seat->destroy();
m_shm->destroy();
m_subCompositor->destroy();
m_queue->destroy();
}
QTEST_GUILESS_MAIN(TestWaylandSeat)
#include "test_wayland_seat.moc"
......@@ -63,6 +63,7 @@ private Q_SLOTS:
void testMove();
void testResize_data();
void testResize();
void testDisconnect();
private:
KWayland::Server::Display *m_display;
......@@ -701,5 +702,44 @@ void TestWaylandShell::testResize()
QTEST(resizeRequestedSpy.first().at(2).value<Qt::Edges>(), "expectedEdge");
}
void TestWaylandShell::testDisconnect()
{
// this test verifies that the server side correctly tears down the resources when the client disconnects
using namespace KWayland::Client;
using namespace KWayland::Server;
QScopedPointer<KWayland::Client::Surface> s(m_compositor->createSurface());
QVERIFY(!s.isNull());
QVERIFY(s->isValid());
QScopedPointer<ShellSurface> surface(m_shell->createSurface(s.data(), m_shell));
QSignalSpy serverSurfaceSpy(m_shellInterface, &ShellInterface::surfaceCreated);
QVERIFY(serverSurfaceSpy.isValid());
QVERIFY(serverSurfaceSpy.wait());
ShellSurfaceInterface *serverSurface = serverSurfaceSpy.first().first().value<ShellSurfaceInterface*>();
QVERIFY(serverSurface);
// destroy client
QSignalSpy clientDisconnectedSpy(serverSurface->client(), &ClientConnection::disconnected);
QVERIFY(clientDisconnectedSpy.isValid());
QSignalSpy shellSurfaceDestroyedSpy(serverSurface, &QObject::destroyed);
QVERIFY(shellSurfaceDestroyedSpy.isValid());
if (m_connection) {
m_connection->deleteLater();
m_connection = nullptr;
}
QVERIFY(clientDisconnectedSpy.wait());
QCOMPARE(clientDisconnectedSpy.count(), 1);
QCOMPARE(shellSurfaceDestroyedSpy.count(), 0);
QVERIFY(shellSurfaceDestroyedSpy.wait());
QCOMPARE(shellSurfaceDestroyedSpy.count(), 1);
s->destroy();
surface->destroy();
m_shell->destroy();
m_compositor->destroy();
m_pointer->destroy();
m_seat->destroy();
m_queue->destroy();
}
QTEST_GUILESS_MAIN(TestWaylandShell)
#include "test_wayland_shell.moc"
......@@ -58,6 +58,7 @@ private Q_SLOTS:
void testDamageTracking();
void testSurfaceAt();
void testDestroyAttachedBuffer();
void testDisconnect();
private:
KWayland::Server::Display *m_display;
......@@ -873,5 +874,40 @@ void TestWaylandSurface::testDestroyAttachedBuffer()
QVERIFY(!serverSurface->buffer());
}
void TestWaylandSurface::testDisconnect()
{
// this test verifies that the server side correctly tears down the resources when the client disconnects
using namespace KWayland::Client;
using namespace KWayland::Server;
QScopedPointer<Surface> s(m_compositor->createSurface());
QVERIFY(!s.isNull());
QVERIFY(s->isValid());
QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated);
QVERIFY(surfaceCreatedSpy.isValid());
QVERIFY(surfaceCreatedSpy.wait());
auto serverSurface = surfaceCreatedSpy.first().first().value<SurfaceInterface*>();
QVERIFY(serverSurface);
// destroy client
QSignalSpy clientDisconnectedSpy(serverSurface->client(), &ClientConnection::disconnected);
QVERIFY(clientDisconnectedSpy.isValid());
QSignalSpy surfaceDestroyedSpy(serverSurface, &QObject::destroyed);
QVERIFY(surfaceDestroyedSpy.isValid());
if (m_connection) {
m_connection->deleteLater();
m_connection = nullptr;
}
QVERIFY(clientDisconnectedSpy.wait());
QCOMPARE(clientDisconnectedSpy.count(), 1);
QCOMPARE(surfaceDestroyedSpy.count(), 0);
QVERIFY(surfaceDestroyedSpy.wait());
QCOMPARE(surfaceDestroyedSpy.count(), 1);
s->destroy();
m_shm->destroy();
m_compositor->destroy();
m_queue->destroy();
}
QTEST_GUILESS_MAIN(TestWaylandSurface)
#include "test_wayland_surface.moc"
......@@ -84,6 +84,8 @@ void TestQtSurfaceExtension::testCloseWindow()
QCOMPARE(surfaceExtensionSpy.count(), 1);
auto *extension = surfaceExtensionSpy.first().first().value<QtExtendedSurfaceInterface*>();
QVERIFY(extension);
QSignalSpy surfaceExtensionDestroyedSpy(extension, &QObject::destroyed);
QVERIFY(surfaceExtensionSpy.isValid());
QSignalSpy processStateChangedSpy(&process, &QProcess::stateChanged);
QVERIFY(processStateChangedSpy.isValid());
extension->close();
......@@ -91,6 +93,10 @@ void TestQtSurfaceExtension::testCloseWindow()
QVERIFY(processStateChangedSpy.wait());
QCOMPARE(process.exitStatus(), QProcess::NormalExit);
if (surfaceExtensionDestroyedSpy.count() == 0) {
QVERIFY(surfaceExtensionDestroyedSpy.wait());
}
QCOMPARE(surfaceExtensionSpy.count(), 1);
}
QTEST_GUILESS_MAIN(TestQtSurfaceExtension)
......
......@@ -105,14 +105,6 @@ void CompositorInterface::Private::createSurface(wl_client *client, wl_resource
delete surface;
return;
}
QObject::connect(surface->client(), &ClientConnection::disconnected, surface,
[surface] {
if (surface->resource()) {
wl_resource_destroy(surface->resource());
delete surface;
}
}
);
emit q->surfaceCreated(surface);
}
......@@ -130,14 +122,6 @@ void CompositorInterface::Private::createRegion(wl_client *client, wl_resource *
delete region;
return;
}
QObject::connect(region->client(), &ClientConnection::disconnected, region,
[region] {
if (region->resource()) {
wl_resource_destroy(region->resource());
delete region;
}
}
);
emit q->regionCreated(region);
}
......
......@@ -98,7 +98,7 @@ void DataOfferInterface::Private::receive(const QString &mimeType, qint32 fd)
}
DataOfferInterface::DataOfferInterface(DataSourceInterface *source, DataDeviceInterface *parentInterface, wl_resource *parentResource)
: Resource(new Private(source, parentInterface, this, parentResource), parentInterface)
: Resource(new Private(source, parentInterface, this, parentResource))
{
connect(source, &DataSourceInterface::mimeTypeOffered, this,
[this](const QString &mimeType) {
......
......@@ -120,7 +120,7 @@ void DpmsInterface::Private::releaseCallback(wl_client *client, wl_resource *res
}
DpmsInterface::DpmsInterface(OutputInterface *output, wl_resource *parentResource, DpmsManagerInterface *manager)
: Resource(new Private(this, manager, parentResource, output), manager)
: Resource(new Private(this, manager, parentResource, output))
{
connect(output, &OutputInterface::dpmsSupportedChanged, this,
[this] {
......
......@@ -81,7 +81,7 @@ const struct wl_keyboard_interface KeyboardInterface::Private::s_interface {
#endif
KeyboardInterface::KeyboardInterface(SeatInterface *parent, wl_resource *parentResource)
: Resource(new Private(parent, parentResource, this), parent)
: Resource(new Private(parent, parentResource, this))
{
}
......
......@@ -164,7 +164,7 @@ const struct org_kde_plasma_surface_interface PlasmaShellSurfaceInterface::Priva
#endif
PlasmaShellSurfaceInterface::PlasmaShellSurfaceInterface(PlasmaShellInterface *shell, SurfaceInterface *parent, wl_resource *parentResource)
: Resource(new Private(this, shell, parent, parentResource), parent)
: Resource(new Private(this, shell, parent, parentResource))
{
}
......
......@@ -105,7 +105,7 @@ const struct wl_pointer_interface PointerInterface::Private::s_interface = {
#endif
PointerInterface::PointerInterface(SeatInterface *parent, wl_resource *parentResource)
: Resource(new Private(parent, parentResource, this), parent)
: Resource(new Private(parent, parentResource, this))
{
// TODO: handle touch
connect(parent, &SeatInterface::pointerPosChanged, this, [this] {
......
......@@ -187,7 +187,7 @@ void QtExtendedSurfaceInterface::Private::updateGenericPropertyCallback(wl_clien
}
QtExtendedSurfaceInterface::QtExtendedSurfaceInterface(QtSurfaceExtensionInterface *shell, SurfaceInterface *parent, wl_resource *parentResource)
: Resource(new Private(this, shell, parent, parentResource), parent)
: Resource(new Private(this, shell, parent, parentResource))
{
}
......
......@@ -100,7 +100,7 @@ void RegionInterface::Private::destroyCallback(wl_client *client, wl_resource *r
}
RegionInterface::RegionInterface(CompositorInterface *parent, wl_resource *parentResource)
: Resource(new Private(parent, this, parentResource), parent)
: Resource(new Private(parent, this, parentResource))
{
}
......
......@@ -189,7 +189,7 @@ const struct wl_shell_surface_interface ShellSurfaceInterface::Private::s_interf
#endif
ShellSurfaceInterface::ShellSurfaceInterface(ShellInterface *shell, SurfaceInterface *parent, wl_resource *parentResource)
: Resource(new Private(this, shell, parent, parentResource), parent)
: Resource(new Private(this, shell, parent, parentResource))
{
Q_D();
connect(d->pingTimer.data(), &QTimer::timeout, this, &ShellSurfaceInterface::pingTimeout);
......
......@@ -70,7 +70,7 @@ void TouchInterface::Private::releaseCallback(wl_client *client, wl_resource *re
}
TouchInterface::TouchInterface(SeatInterface *parent, wl_resource *parentResource)
: Resource(new Private(parent, parentResource, this), parent)
: Resource(new Private(parent, parentResource, this))
{
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment