Don't crash if a client (legally) uses deleted global contrast manager

Summary:
There is a race condition in the following situation:

    - Server creates a global
    - Client binds to that global (making a new resource for that
global)
    Simultaneously:
    - The client uses this resource
    - The server deletes the global

This was fixed for Blur, but as mention in that commit can also happen here.
Code is effectively a copy and paste from e8850b014c

Test Plan: Unit test. Booted normal session

Reviewers: #plasma

Subscribers: plasma-devel, #frameworks

Tags: #plasma_on_wayland, #frameworks

Differential Revision: https://phabricator.kde.org/D7885
This commit is contained in:
David Edmundson 2017-09-20 14:16:26 +01:00
parent e8850b014c
commit 01cace30dd
2 changed files with 38 additions and 8 deletions

View file

@ -21,6 +21,7 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>.
#include <QtTest/QtTest>
// KWin
#include "../../src/client/blur.h"
#include "../../src/client/contrast.h"
#include "../../src/client/compositor.h"
#include "../../src/client/connection_thread.h"
#include "../../src/client/dpms.h"
@ -120,6 +121,8 @@ private:
KWayland::Server::PointerGesturesInterface *m_pointerGesturesV1;
KWayland::Server::PointerConstraintsInterface *m_pointerConstraintsV1;
KWayland::Server::BlurManagerInterface *m_blur;
KWayland::Server::ContrastManagerInterface *m_contrast;
};
static const QString s_socketName = QStringLiteral("kwin-test-wayland-registry-0");
@ -143,6 +146,7 @@ TestWaylandRegistry::TestWaylandRegistry(QObject *parent)
, m_pointerGesturesV1(nullptr)
, m_pointerConstraintsV1(nullptr)
, m_blur(nullptr)
, m_contrast(nullptr)
{
}
@ -171,7 +175,8 @@ void TestWaylandRegistry::init()
QVERIFY(m_outputManagement->isValid());
m_blur = m_display->createBlurManager(this);
m_blur->create();
m_display->createContrastManager(this)->create();
m_contrast = m_display->createContrastManager(this);
m_contrast->create();
m_display->createSlideManager(this)->create();
m_display->createDpmsManager()->create();
m_serverSideDecorationManager = m_display->createServerSideDecorationManager();
@ -581,18 +586,22 @@ void TestWaylandRegistry::testOutOfSyncRemoval()
registry.create(connection.display());
registry.setup();
QSignalSpy blurAnnouncedSpy(&registry, &Registry::blurAnnounced);
QSignalSpy contrastAnnouncedSpy(&registry, &Registry::blurAnnounced);
blurAnnouncedSpy.wait();
contrastAnnouncedSpy.wait();
BlurManager *blurManager = registry.createBlurManager(registry.interface(Registry::Interface::Blur).name, registry.interface(Registry::Interface::Blur).version, &registry);
ContrastManager *contrastManager = registry.createContrastManager(registry.interface(Registry::Interface::Contrast).name, registry.interface(Registry::Interface::Contrast).version, &registry);
connection.flush();
m_display->dispatchEvents();
QScopedPointer<Compositor> compositor(registry.createCompositor(registry.interface(Registry::Interface::Compositor).name,
registry.interface(Registry::Interface::Compositor).version));
QScopedPointer<Surface> surface(compositor->createSurface());
QVERIFY(surface);
//remove blur
QSignalSpy blurRemovedSpy(&registry, &Registry::blurRemoved);
delete m_blur;
@ -607,6 +616,20 @@ void TestWaylandRegistry::testOutOfSyncRemoval()
QVERIFY(blurRemovedSpy.wait());
QVERIFY(blurRemovedSpy.count() == 1);
//remove background contrast
QSignalSpy contrastRemovedSpy(&registry, &Registry::contrastRemoved);
delete m_contrast;
//client hasn't processed the event yet
QVERIFY(contrastRemovedSpy.count() == 0);
//use the in the client
contrastManager->createContrast(surface.data(), 0);
//now process events,
QVERIFY(contrastRemovedSpy.wait());
QVERIFY(contrastRemovedSpy.count() == 1);
}

View file

@ -46,7 +46,11 @@ private:
static void unsetCallback(wl_client *client, wl_resource *resource, wl_resource *surface);
static void unbind(wl_resource *resource);
static Private *cast(wl_resource *r) {
return reinterpret_cast<Private*>(wl_resource_get_user_data(r));
auto contrastManager = reinterpret_cast<QPointer<ContrastManagerInterface>*>(wl_resource_get_user_data(r))->data();
if (contrastManager) {
return static_cast<Private*>(contrastManager->d.data());
}
return nullptr;
}
ContrastManagerInterface *q;
@ -77,19 +81,22 @@ void ContrastManagerInterface::Private::bind(wl_client *client, uint32_t version
wl_client_post_no_memory(client);
return;
}
wl_resource_set_implementation(resource, &s_interface, this, unbind);
// TODO: should we track?
auto ref = new QPointer<ContrastManagerInterface>(q);//deleted in unbind
wl_resource_set_implementation(resource, &s_interface, ref, unbind);
}
void ContrastManagerInterface::Private::unbind(wl_resource *resource)
{
Q_UNUSED(resource)
// TODO: implement?
delete reinterpret_cast<QPointer<ContrastManagerInterface>*>(wl_resource_get_user_data(resource));
}
void ContrastManagerInterface::Private::createCallback(wl_client *client, wl_resource *resource, uint32_t id, wl_resource *surface)
{
cast(resource)->createContrast(client, resource, id, surface);
auto m = cast(resource);
if (!m) {
return;// will happen if global is deleted
}
m->createContrast(client, resource, id, surface);
}
void ContrastManagerInterface::Private::createContrast(wl_client *client, wl_resource *resource, uint32_t id, wl_resource *surface)