Destroy all clients before destroying wl_display

One of the most disappointing things when writing autotests is dealing
with a race condition where destructor requests are processed after all
globals have been destroyed.

With this change, the Display object will destroy all clients and their
resources before destroying the wl_display object. The good thing about
doing so is that shut down logic becomes simple. We don't have to assume
that wl_resource objects can outlive their wl_global objects, etc. The
bad thing is that it exposed a couple of pre-existing latent bugs in the
data device and the xdg foreign code.

closes plasma/kwayland-server#2
This commit is contained in:
Vlad Zahorodnii 2020-10-30 15:36:24 +02:00
parent ed83cea823
commit 34982850e2
23 changed files with 135 additions and 203 deletions

View file

@ -113,10 +113,11 @@ void ErrorTest::cleanup()
delete m_thread;
m_thread = nullptr;
}
CLEANUP(m_psi)
CLEANUP(m_ci)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_psi = nullptr;
m_ci = nullptr;
}
void ErrorTest::testMultiplePlasmaShellSurfacesForSurface()

View file

@ -122,9 +122,11 @@ void FakeInputTest::cleanup()
}
CLEANUP(m_device)
CLEANUP(m_fakeInputInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_fakeInputInterface = nullptr;
}
void FakeInputTest::testAuthenticate()

View file

@ -114,10 +114,12 @@ void IdleTest::cleanup()
m_thread = nullptr;
}
CLEANUP(m_idleInterface)
CLEANUP(m_seatInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_idleInterface = nullptr;
m_seatInterface = nullptr;
}
void IdleTest::testTimeout()

View file

@ -145,7 +145,6 @@ void PlasmaWindowModelTest::cleanup()
variable = nullptr; \
}
CLEANUP(m_pw)
CLEANUP(m_plasmaVirtualDesktopManagementInterface)
CLEANUP(m_queue)
if (m_connection) {
m_connection->deleteLater();
@ -158,9 +157,12 @@ void PlasmaWindowModelTest::cleanup()
m_thread = nullptr;
}
CLEANUP(m_pwInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_plasmaVirtualDesktopManagementInterface = nullptr;
m_pwInterface = nullptr;
}
bool PlasmaWindowModelTest::testBooleanData(PlasmaWindowModel::AdditionalRoles role, void (PlasmaWindowInterface::*function)(bool))

View file

@ -142,11 +142,13 @@ void TestPointerConstraints::cleanup()
m_thread = nullptr;
}
CLEANUP(m_compositorInterface)
CLEANUP(m_seatInterface);
CLEANUP(m_pointerConstraintsInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_compositorInterface = nullptr;
m_seatInterface = nullptr;
m_pointerConstraintsInterface = nullptr;
}
void TestPointerConstraints::testLockPointer_data()

View file

@ -162,11 +162,12 @@ void SelectionTest::cleanup()
delete variable; \
variable = nullptr;
CLEANUP(m_ddmInterface)
CLEANUP(m_seatInterface)
CLEANUP(m_compositorInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_ddmInterface = nullptr;
m_seatInterface = nullptr;
m_compositorInterface = nullptr;
}
void SelectionTest::cleanupConnection(Connection *c)

View file

@ -120,10 +120,12 @@ void ShadowTest::cleanup()
m_thread = nullptr;
}
CLEANUP(m_compositorInterface)
CLEANUP(m_shadowInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_compositorInterface = nullptr;
m_shadowInterface = nullptr;
}
void ShadowTest::testCreateShadow()

View file

@ -153,11 +153,13 @@ void TextInputTest::cleanup()
m_thread = nullptr;
}
CLEANUP(m_textInputManagerV2Interface)
CLEANUP(m_compositorInterface)
CLEANUP(m_seatInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_textInputManagerV2Interface = nullptr;
m_compositorInterface = nullptr;
m_seatInterface = nullptr;
}
SurfaceInterface *TextInputTest::waitForSurface()

View file

@ -126,10 +126,12 @@ void TestBlur::cleanup()
delete m_thread;
m_thread = nullptr;
}
CLEANUP(m_compositorInterface)
CLEANUP(m_blurManagerInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_compositorInterface = nullptr;
m_blurManagerInterface = nullptr;
}
void TestBlur::testCreate()

View file

@ -129,10 +129,12 @@ void TestContrast::cleanup()
delete m_thread;
m_thread = nullptr;
}
CLEANUP(m_compositorInterface)
CLEANUP(m_contrastManagerInterface)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_compositorInterface = nullptr;
m_contrastManagerInterface = nullptr;
}
void TestContrast::testCreate()

View file

@ -118,11 +118,11 @@ void TestWaylandOutput::cleanup()
delete m_connection;
m_connection = nullptr;
delete m_serverOutput;
m_serverOutput = nullptr;
delete m_display;
m_display = nullptr;
// these are the children of the display
m_serverOutput = nullptr;
}
void TestWaylandOutput::testRegistry()

View file

@ -200,14 +200,13 @@ void TestWaylandOutputManagement::cleanup()
m_thread = nullptr;
}
if (m_outputManagementInterface) {
delete m_outputManagementInterface;
m_outputManagementInterface = nullptr;
}
delete m_display;
m_display = nullptr;
m_serverOutputs.clear();
m_clientOutputs.clear();
// these are the children of the display
m_outputManagementInterface = nullptr;
}
void TestWaylandOutputManagement::applyPendingChanges(KWaylandServer::OutputConfigurationInterface *configurationInterface)

View file

@ -236,23 +236,15 @@ void TestWaylandSeat::cleanup()
m_thread = nullptr;
}
delete m_compositorInterface;
m_compositorInterface = nullptr;
delete m_seatInterface;
m_seatInterface = nullptr;
delete m_subCompositorInterface;
m_subCompositorInterface = nullptr;
delete m_relativePointerManagerV1Interface;
m_relativePointerManagerV1Interface = nullptr;
delete m_pointerGesturesV1Interface;
m_pointerGesturesV1Interface = nullptr;
delete m_display;
m_display = nullptr;
// these are the children of the display
m_compositorInterface = nullptr;
m_seatInterface = nullptr;
m_subCompositorInterface = nullptr;
m_relativePointerManagerV1Interface = nullptr;
m_pointerGesturesV1Interface = nullptr;
}
void TestWaylandSeat::testName()

View file

@ -169,14 +169,12 @@ void TestWaylandSurface::cleanup()
delete m_connection;
m_connection = nullptr;
delete m_compositorInterface;
m_compositorInterface = nullptr;
delete m_idleInhibitInterface;
m_idleInhibitInterface = nullptr;
delete m_display;
m_display = nullptr;
// these are the children of the display
m_compositorInterface = nullptr;
m_idleInhibitInterface = nullptr;
}
void TestWaylandSurface::testStaticAccessor()

View file

@ -223,16 +223,15 @@ void TestWindowManagement::cleanup()
m_connection = nullptr;
m_display->dispatchEvents();
delete m_windowManagementInterface;
m_windowManagementInterface = nullptr;
delete m_windowInterface;
m_windowInterface = nullptr;
QVERIFY(m_surfaceInterface.isNull());
delete m_display;
m_display = nullptr;
// these are the children of the display
m_windowManagementInterface = nullptr;
m_windowInterface = nullptr;
}
void TestWindowManagement::testUseAfterUnmap()

View file

@ -159,11 +159,12 @@ void TestForeign::cleanup()
delete m_thread;
m_thread = nullptr;
}
CLEANUP(m_foreignInterface)
delete m_display;
m_display = nullptr;
#undef CLEANUP
// these are the children of the display
m_foreignInterface = nullptr;
}
void TestForeign::doExport()

View file

@ -189,13 +189,15 @@ void XdgShellTest::cleanup()
m_thread = nullptr;
}
CLEANUP(m_compositorInterface)
CLEANUP(m_xdgShellInterface)
CLEANUP(m_o1Interface);
CLEANUP(m_o2Interface);
CLEANUP(m_seatInterface);
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_compositorInterface = nullptr;
m_xdgShellInterface = nullptr;
m_o1Interface = nullptr;
m_o2Interface = nullptr;
m_seatInterface = nullptr;
}
void XdgShellTest::testCreateSurface()

View file

@ -231,10 +231,12 @@ void DataControlInterfaceTest::cleanup()
delete m_thread;
m_thread = nullptr;
}
CLEANUP(m_seat)
CLEANUP(m_serverCompositor)
CLEANUP(m_display)
#undef CLEANUP
// these are the children of the display
m_seat = nullptr;
m_serverCompositor = nullptr;
}
void DataControlInterfaceTest::testCopyToControl()

View file

@ -21,7 +21,6 @@ private Q_SLOTS:
void testName();
void testPointerButton();
void testPointerPos();
void testDestroyThroughTerminate();
void testRepeatInfo();
void testMultiple();
};
@ -155,18 +154,6 @@ void TestWaylandServerSeat::testPointerPos()
QCOMPARE(seatPosSpy.last().first().toPointF(), QPointF(5, 7));
}
void TestWaylandServerSeat::testDestroyThroughTerminate()
{
QScopedPointer<Display> display(new Display());
display->setSocketName(s_socketName);
display->start();
SeatInterface *seat = display->createSeat();
QSignalSpy destroyedSpy(seat, SIGNAL(destroyed(QObject*)));
QVERIFY(destroyedSpy.isValid());
display.reset();
QVERIFY(!destroyedSpy.isEmpty());
}
void TestWaylandServerSeat::testRepeatInfo()
{
Display display;

View file

@ -116,7 +116,7 @@ Display::Display(QObject *parent)
Display::~Display()
{
emit aboutToTerminate();
wl_display_destroy_clients(d->display);
wl_display_destroy(d->display);
}
@ -216,302 +216,219 @@ OutputInterface *Display::createOutput(QObject *parent)
{
OutputInterface *output = new OutputInterface(this, parent);
connect(output, &QObject::destroyed, this, [this,output] { d->outputs.removeAll(output); });
connect(this, &Display::aboutToTerminate, output, [this,output] { removeOutput(output); });
d->outputs << output;
return output;
}
CompositorInterface *Display::createCompositor(QObject *parent)
{
CompositorInterface *compositor = new CompositorInterface(this, parent);
connect(this, &Display::aboutToTerminate, compositor, [compositor] { delete compositor; });
return compositor;
return new CompositorInterface(this, parent);
}
OutputDeviceInterface *Display::createOutputDevice(QObject *parent)
{
OutputDeviceInterface *output = new OutputDeviceInterface(this, parent);
connect(output, &QObject::destroyed, this, [this,output] { d->outputdevices.removeAll(output); });
connect(this, &Display::aboutToTerminate, output, [this,output] { removeOutputDevice(output); });
d->outputdevices << output;
return output;
}
OutputManagementInterface *Display::createOutputManagement(QObject *parent)
{
OutputManagementInterface *om = new OutputManagementInterface(this, parent);
connect(this, &Display::aboutToTerminate, om, [om] { delete om; });
return om;
return new OutputManagementInterface(this, parent);
}
SeatInterface *Display::createSeat(QObject *parent)
{
SeatInterface *seat = new SeatInterface(this, parent);
connect(seat, &QObject::destroyed, this, [this, seat] { d->seats.removeAll(seat); });
connect(this, &Display::aboutToTerminate, seat, [seat] { delete seat; });
d->seats << seat;
return seat;
}
SubCompositorInterface *Display::createSubCompositor(QObject *parent)
{
auto c = new SubCompositorInterface(this, parent);
connect(this, &Display::aboutToTerminate, c, [c] { delete c; });
return c;
return new SubCompositorInterface(this, parent);
}
DataDeviceManagerInterface *Display::createDataDeviceManager(QObject *parent)
{
auto m = new DataDeviceManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, m, [m] { delete m; });
return m;
return new DataDeviceManagerInterface(this, parent);
}
PlasmaShellInterface *Display::createPlasmaShell(QObject* parent)
{
auto s = new PlasmaShellInterface(this, parent);
connect(this, &Display::aboutToTerminate, s, [s] { delete s; });
return s;
return new PlasmaShellInterface(this, parent);
}
PlasmaWindowManagementInterface *Display::createPlasmaWindowManagement(QObject *parent)
{
auto wm = new PlasmaWindowManagementInterface(this, parent);
connect(this, &Display::aboutToTerminate, wm, [wm] { delete wm; });
return wm;
return new PlasmaWindowManagementInterface(this, parent);
}
IdleInterface *Display::createIdle(QObject *parent)
{
auto i = new IdleInterface(this, parent);
connect(this, &Display::aboutToTerminate, i, [i] { delete i; });
return i;
return new IdleInterface(this, parent);
}
FakeInputInterface *Display::createFakeInput(QObject *parent)
{
auto i = new FakeInputInterface(this, parent);
connect(this, &Display::aboutToTerminate, i, [i] { delete i; });
return i;
return new FakeInputInterface(this, parent);
}
ShadowManagerInterface *Display::createShadowManager(QObject *parent)
{
auto s = new ShadowManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, s, [s] { delete s; });
return s;
return new ShadowManagerInterface(this, parent);
}
BlurManagerInterface *Display::createBlurManager(QObject *parent)
{
auto b = new BlurManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new BlurManagerInterface(this, parent);
}
ContrastManagerInterface *Display::createContrastManager(QObject *parent)
{
auto b = new ContrastManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new ContrastManagerInterface(this, parent);
}
SlideManagerInterface *Display::createSlideManager(QObject *parent)
{
auto b = new SlideManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new SlideManagerInterface(this, parent);
}
DpmsManagerInterface *Display::createDpmsManager(QObject *parent)
{
auto d = new DpmsManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new DpmsManagerInterface(this, parent);
}
ServerSideDecorationManagerInterface *Display::createServerSideDecorationManager(QObject *parent)
{
auto d = new ServerSideDecorationManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new ServerSideDecorationManagerInterface(this, parent);
}
ScreencastV1Interface *Display::createScreencastV1Interface(QObject *parent)
{
auto s = new ScreencastV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, s, [s] { delete s; });
return s;
return new ScreencastV1Interface(this, parent);
}
TextInputManagerV2Interface *Display::createTextInputManagerV2(QObject *parent)
{
auto t = new TextInputManagerV2Interface(this, parent);
connect(this, &Display::aboutToTerminate, t, [t] { delete t; });
return t;
return new TextInputManagerV2Interface(this, parent);
}
TextInputManagerV3Interface *Display::createTextInputManagerV3(QObject *parent)
{
auto t = new TextInputManagerV3Interface(this, parent);
connect(this, &Display::aboutToTerminate, t, [t] { delete t; });
return t;
return new TextInputManagerV3Interface(this, parent);
}
XdgShellInterface *Display::createXdgShell(QObject *parent)
{
XdgShellInterface *shell = new XdgShellInterface(this, parent);
connect(this, &Display::aboutToTerminate, shell, [shell] { delete shell; });
return shell;
return new XdgShellInterface(this, parent);
}
RelativePointerManagerV1Interface *Display::createRelativePointerManagerV1(QObject *parent)
{
RelativePointerManagerV1Interface *r = new RelativePointerManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, r, [r] { delete r; });
return r;
return new RelativePointerManagerV1Interface(this, parent);
}
PointerGesturesV1Interface *Display::createPointerGesturesV1(QObject *parent)
{
PointerGesturesV1Interface *p = new PointerGesturesV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, p, [p] { delete p; });
return p;
return new PointerGesturesV1Interface(this, parent);
}
PointerConstraintsV1Interface *Display::createPointerConstraintsV1(QObject *parent)
{
PointerConstraintsV1Interface *p = new PointerConstraintsV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, p, [p] { delete p; });
return p;
return new PointerConstraintsV1Interface(this, parent);
}
XdgForeignV2Interface *Display::createXdgForeignV2Interface(QObject *parent)
{
XdgForeignV2Interface *foreign = new XdgForeignV2Interface(this, parent);
connect(this, &Display::aboutToTerminate, foreign, [foreign] { delete foreign; });
return foreign;
return new XdgForeignV2Interface(this, parent);
}
IdleInhibitManagerV1Interface *Display::createIdleInhibitManagerV1(QObject *parent)
{
IdleInhibitManagerV1Interface *idleManager = new IdleInhibitManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, idleManager, [idleManager] { delete idleManager; });
return idleManager;
return new IdleInhibitManagerV1Interface(this, parent);
}
AppMenuManagerInterface *Display::createAppMenuManagerInterface(QObject *parent)
{
auto b = new AppMenuManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new AppMenuManagerInterface(this, parent);
}
ServerSideDecorationPaletteManagerInterface *Display::createServerSideDecorationPaletteManager(QObject *parent)
{
auto b = new ServerSideDecorationPaletteManagerInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new ServerSideDecorationPaletteManagerInterface(this, parent);
}
LinuxDmabufUnstableV1Interface *Display::createLinuxDmabufInterface(QObject *parent)
{
auto b = new LinuxDmabufUnstableV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new LinuxDmabufUnstableV1Interface(this, parent);
}
PlasmaVirtualDesktopManagementInterface *Display::createPlasmaVirtualDesktopManagement(QObject *parent)
{
auto b = new PlasmaVirtualDesktopManagementInterface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new PlasmaVirtualDesktopManagementInterface(this, parent);
}
XdgOutputManagerV1Interface *Display::createXdgOutputManagerV1(QObject *parent)
{
auto b = new XdgOutputManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, b, [b] { delete b; });
return b;
return new XdgOutputManagerV1Interface(this, parent);
}
XdgDecorationManagerV1Interface *Display::createXdgDecorationManagerV1(QObject *parent)
{
auto d = new XdgDecorationManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new XdgDecorationManagerV1Interface(this, parent);
}
EglStreamControllerInterface *Display::createEglStreamControllerInterface(QObject *parent)
{
EglStreamControllerInterface *e = new EglStreamControllerInterface(this, parent);
connect(this, &Display::aboutToTerminate, e, [e] { delete e; });
return e;
return new EglStreamControllerInterface(this, parent);
}
KeyStateInterface *Display::createKeyStateInterface(QObject *parent)
{
auto d = new KeyStateInterface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new KeyStateInterface(this, parent);
}
TabletManagerV2Interface *Display::createTabletManagerV2(QObject *parent)
{
auto d = new TabletManagerV2Interface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new TabletManagerV2Interface(this, parent);
}
DataControlDeviceManagerV1Interface *Display::createDataControlDeviceManagerV1(QObject *parent)
{
auto m = new DataControlDeviceManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, m, [m] { delete m; });
return m;
return new DataControlDeviceManagerV1Interface(this, parent);
}
KeyboardShortcutsInhibitManagerV1Interface *Display::createKeyboardShortcutsInhibitManagerV1(QObject *parent)
{
auto d = new KeyboardShortcutsInhibitManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new KeyboardShortcutsInhibitManagerV1Interface(this, parent);
}
InputMethodV1Interface *Display::createInputMethodInterface(QObject *parent)
{
auto d = new InputMethodV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, d, [d] { delete d; });
return d;
return new InputMethodV1Interface(this, parent);
}
ViewporterInterface *Display::createViewporter(QObject *parent)
{
auto viewporter = new ViewporterInterface(this, parent);
connect(this, &Display::aboutToTerminate, viewporter, [viewporter] { delete viewporter; });
return viewporter;
return new ViewporterInterface(this, parent);
}
PrimarySelectionDeviceManagerV1Interface *Display::createPrimarySelectionDeviceManagerV1(QObject *parent)
{
auto primarySelection = new PrimarySelectionDeviceManagerV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, primarySelection, [primarySelection] { delete primarySelection; });
return primarySelection;
return new PrimarySelectionDeviceManagerV1Interface(this, parent);
}
InputPanelV1Interface *Display::createInputPanelInterface(QObject *parent)
{
auto p = new InputPanelV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, p, [p] { delete p; });
return p;
return new InputPanelV1Interface(this, parent);
}
LayerShellV1Interface *Display::createLayerShellV1(QObject *parent)
{
auto shell = new LayerShellV1Interface(this, parent);
connect(this, &Display::aboutToTerminate, shell, [shell] { delete shell; });
return shell;
return new LayerShellV1Interface(this, parent);
}
void Display::createShm()

View file

@ -378,7 +378,6 @@ Q_SIGNALS:
void socketNameChanged(const QString&);
void automaticSocketNamingChanged(bool);
void runningChanged(bool);
void aboutToTerminate();
void clientConnected(KWaylandServer::ClientConnection*);
void clientDisconnected(KWaylandServer::ClientConnection*);

View file

@ -33,15 +33,28 @@ void Global::Private::create()
global = wl_global_create(*display, m_interface, m_version, this, bind);
}
static void handleDisplayDestroyed(struct wl_listener *listener, void *data)
{
Q_UNUSED(data)
Global *global = static_cast<DisplayDestroyListener *>(listener)->global;
global->destroy();
}
Global::Global(Global::Private *d, QObject *parent)
: QObject(parent)
, d(d)
{
d->displayDestroyListener.notify = handleDisplayDestroyed;
d->displayDestroyListener.global = this;
d->displayDestroyListener.link.next = nullptr;
d->displayDestroyListener.link.prev = nullptr;
wl_display_add_destroy_listener(*d->display, &d->displayDestroyListener);
}
Global::~Global()
{
destroy();
wl_list_remove(&d->displayDestroyListener.link);
}
void Global::create()

View file

@ -8,12 +8,16 @@
#include "global.h"
struct wl_client;
struct wl_interface;
#include <wayland-server-core.h>
namespace KWaylandServer
{
struct DisplayDestroyListener : public wl_listener
{
Global *global = nullptr;
};
class Global::Private
{
public:
@ -23,6 +27,7 @@ public:
Display *display = nullptr;
wl_global *global = nullptr;
DisplayDestroyListener displayDestroyListener;
protected:
Private(Display *d, const wl_interface *interface, quint32 version);