yukijoou/kwin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[server] Prevent double delete of callback resources in SurfaceInterface

Browse source 
Summary:
When destroying a SurfaceInterface all callbacks are getting destroyed.
This used to iterate over the callbacks and performing
wl_resource_destroy on them. This triggered the destroy handler which
removes the resource from the callback list. Which means removing from
the list we are iterating on. This could result in a double delete or
accessing invalid memory.

This change copies all callbacks to a temporary list and clears the
normal lists. So the destroy handler does no longer modify the lists
currently being iterated on.

Test Plan: Added a test case which crashed with previous code

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D1677
This commit is contained in:
Martin Gräßlin 2016-05-25 08:59:10 +02:00
parent 46cf9fc36c
commit d1a09838e1
2 changed files with 51 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
src/wayland/autotests/client/test_wayland_surface.cpp
View file

@ -58,6 +58,7 @@ private Q_SLOTS:
void testDamageTracking();
void testSurfaceAt();
void testDestroyAttachedBuffer();
void testDestroyWithPendingCallback();
void testDisconnect();
private:
@ -874,6 +875,44 @@ void TestWaylandSurface::testDestroyAttachedBuffer()
QVERIFY(!serverSurface->buffer());
}
void TestWaylandSurface::testDestroyWithPendingCallback()
{
// this test tries to verify that destroying a surface with a pending callback works correctly
// first create surface
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);
// now render to it
QImage img(QSize(10, 10), QImage::Format_ARGB32);
img.fill(Qt::black);
auto b = m_shm->createBuffer(img);
s->attachBuffer(b);
s->damage(QRect(0, 0, 10, 10));
// add some frame callbacks
for (int i = 0; i < 1000; i++) {
wl_surface_frame(*s);
}
s->commit(KWayland::Client::Surface::CommitFlag::FrameCallback);
QSignalSpy damagedSpy(serverSurface, &SurfaceInterface::damaged);
QVERIFY(damagedSpy.isValid());
QVERIFY(damagedSpy.wait());
// now try to destroy the Surface again
QSignalSpy destroyedSpy(serverSurface, &QObject::destroyed);
QVERIFY(destroyedSpy.isValid());
s.reset();
QVERIFY(destroyedSpy.wait());
}
void TestWaylandSurface::testDisconnect()
{
// this test verifies that the server side correctly tears down the resources when the client disconnects

20
src/wayland/surface_interface.cpp
View file

@ -219,14 +219,18 @@ void SurfaceInterface::frameRendered(quint32 msec)
void SurfaceInterface::Private::destroy()
{
for (wl_resource *c : current.callbacks) {
wl_resource_destroy(c);
}
for (wl_resource *c : pending.callbacks) {
wl_resource_destroy(c);
}
for (wl_resource *c : subSurfacePending.callbacks) {
wl_resource_destroy(c);
// copy all existing callbacks to new list and clear existing lists
// the wl_resource_destroy on the callback resource goes into destroyFrameCallback
// which would modify the list we are iterating on
QList<wl_resource *> callbacksToDestroy;
callbacksToDestroy << current.callbacks;
current.callbacks.clear();
callbacksToDestroy << pending.callbacks;
pending.callbacks.clear();
callbacksToDestroy << subSurfacePending.callbacks;
subSurfacePending.callbacks.clear();
for (auto it = callbacksToDestroy.constBegin(), end = callbacksToDestroy.constEnd(); it != end; it++) {
wl_resource_destroy(*it);
}
if (current.buffer) {
current.buffer->unref();