wayland: Remove zombie ClientConnection from Display later

Otherwise it's theoretically possible to create a new ClientConnection
object for the zombie wl_client when its resources are being destroyed.
For example

- process early wl_client destroy notification
- the ClientConnection objects gets removed from the client list in Display
- process wl_resource objects getting destroyed
- if some code calls display->getConnection(zombie_client), it's going to
  reintroduce the client in the client list
- process late wl_client destroy notification, it's going to destroy the
  original and the clone ClientConnection object

This change prevents reintoducing a clone client object, by keeping the
original for a bit longer until it's actually destroyed. In the future though,
it would be great to kill the client lists in Display and ClientConnection,
and just use `static_cast<ClientConnection *>(wl_client_get_user_data())`.
This commit is contained in:
Vlad Zahorodnii 2024-03-20 13:09:23 +02:00
parent 44b72823c2
commit 442648edc0
3 changed files with 4 additions and 18 deletions

View file

@ -81,7 +81,6 @@ void TestWaylandServerDisplay::testClientConnection()
QVERIFY(client);
QVERIFY(connectedSpy.isEmpty());
QVERIFY(display.connections().isEmpty());
ClientConnection *connection = display.getConnection(client);
QVERIFY(connection);
QCOMPARE(connection->client(), client);
@ -101,8 +100,6 @@ void TestWaylandServerDisplay::testClientConnection()
QCOMPARE((wl_client *)constRef, client);
QCOMPARE(connectedSpy.count(), 1);
QCOMPARE(connectedSpy.first().first().value<ClientConnection *>(), connection);
QCOMPARE(display.connections().count(), 1);
QCOMPARE(display.connections().first(), connection);
QCOMPARE(connection, display.getConnection(client));
QCOMPARE(connectedSpy.count(), 1);
@ -119,10 +116,6 @@ void TestWaylandServerDisplay::testClientConnection()
QCOMPARE(connectedSpy.first().first().value<ClientConnection *>(), connection);
QCOMPARE(connectedSpy.last().first().value<ClientConnection *>(), connection2);
QCOMPARE(connectedSpy.last().first().value<ClientConnection *>(), client2);
QCOMPARE(display.connections().count(), 2);
QCOMPARE(display.connections().first(), connection);
QCOMPARE(display.connections().last(), connection2);
QCOMPARE(display.connections().last(), client2);
// and destroy
QVERIFY(disconnectedSpy.isEmpty());
@ -136,7 +129,6 @@ void TestWaylandServerDisplay::testClientConnection()
close(sv[1]);
close(sv2[0]);
close(sv2[1]);
QVERIFY(display.connections().isEmpty());
}
void TestWaylandServerDisplay::testConnectNoSocket()

View file

@ -199,6 +199,7 @@ QList<SeatInterface *> Display::seats() const
ClientConnection *Display::getConnection(wl_client *client)
{
// TODO: Use wl_client_set_user_data() when we start requiring libwayland-server that has it, and remove client lists here and in ClientConnection.
Q_ASSERT(client);
auto it = std::find_if(d->clients.constBegin(), d->clients.constEnd(), [client](ClientConnection *c) {
return c->client() == client;
@ -210,21 +211,15 @@ ClientConnection *Display::getConnection(wl_client *client)
auto c = new ClientConnection(client, this);
d->clients << c;
connect(c, &ClientConnection::disconnected, this, [this](ClientConnection *c) {
const int index = d->clients.indexOf(c);
Q_ASSERT(index != -1);
d->clients.remove(index);
Q_ASSERT(d->clients.indexOf(c) == -1);
Q_EMIT clientDisconnected(c);
});
connect(c, &ClientConnection::destroyed, this, [this, c]() {
d->clients.removeOne(c);
});
Q_EMIT clientConnected(c);
return c;
}
QList<ClientConnection *> Display::connections() const
{
return d->clients;
}
ClientConnection *Display::createClient(int fd)
{
Q_ASSERT(fd != -1);

View file

@ -110,7 +110,6 @@ public:
* @return The ClientConnection for the given native client
*/
ClientConnection *getConnection(wl_client *client);
QList<ClientConnection *> connections() const;
/**
* Returns the graphics buffer for the given @a resource, or @c null if there's no buffer.