From 67521b975b5093c0ae0504c2cc4068de38b6881c Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Fri, 5 May 2017 16:43:35 +0200 Subject: [PATCH] Validate surface is valid when sending TextInput leave event Summary: It's possible for the surface to be unbound when we send the leave event; we've called Resource::unbind() of Surface, so the Surface has, deleteLater called, but it's still a valid object, and the first check passes. We get in this situation because when a surface is destroyed, we're handling text input from the same source event. Sending a nullpointer is a protocol error, and wayland kindly closes the connection. This fixes my constant: "Did the Wayland server die" error messages when running clients. Test Plan: Got errors after setting up qt virtual keyboard. Had reproducible case. Restarted kwin after this patch, now doesn't crash. Reviewers: #plasma, graesslin Subscribers: apol, graesslin, plasma-devel, #frameworks Tags: #plasma_on_wayland, #frameworks Differential Revision: https://phabricator.kde.org/D5712 --- src/wayland/autotests/client/test_text_input.cpp | 15 +++++++++++++++ src/wayland/server/textinput_interface_v2.cpp | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/wayland/autotests/client/test_text_input.cpp b/src/wayland/autotests/client/test_text_input.cpp index 0f6f748fac..9b11aeb392 100644 --- a/src/wayland/autotests/client/test_text_input.cpp +++ b/src/wayland/autotests/client/test_text_input.cpp @@ -306,6 +306,21 @@ void TextInputTest::testEnterLeave() QCOMPARE(textInputChangedSpy.count(), 3); // should still be the same text input QCOMPARE(m_seatInterface->focusedTextInput(), serverTextInput); + //reset + textInput->enable(surface.data()); + QVERIFY(enabledChangedSpy.wait()); + + //trigger an enter again and leave, but this + //time we try sending an event after the surface is unbound + //but not yet destroyed. It should work without errors + QCOMPARE(textInput->enteredSurface(), surface.data()); + connect(serverSurface, &Resource::unbound, [=]() { + m_seatInterface->setFocusedKeyboardSurface(nullptr); + }); + //delete the client and wait for the server to catch up + QSignalSpy unboundSpy(serverSurface, &QObject::destroyed); + surface.reset(); + QVERIFY(unboundSpy.wait()); } void TextInputTest::testShowHidePanel_data() diff --git a/src/wayland/server/textinput_interface_v2.cpp b/src/wayland/server/textinput_interface_v2.cpp index 57a8fc0570..391600230a 100644 --- a/src/wayland/server/textinput_interface_v2.cpp +++ b/src/wayland/server/textinput_interface_v2.cpp @@ -96,7 +96,7 @@ void TextInputUnstableV2Interface::Private::requestDeactivate(SeatInterface *sea void TextInputUnstableV2Interface::Private::sendEnter(SurfaceInterface *surface, quint32 serial) { - if (!resource) { + if (!resource || !surface || !surface->resource()) { return; } zwp_text_input_v2_send_enter(resource, serial, surface->resource()); @@ -104,7 +104,7 @@ void TextInputUnstableV2Interface::Private::sendEnter(SurfaceInterface *surface, void TextInputUnstableV2Interface::Private::sendLeave(quint32 serial, SurfaceInterface *surface) { - if (!resource || !surface) { + if (!resource || !surface || !surface->resource()) { return; } zwp_text_input_v2_send_leave(resource, serial, surface->resource());