wayland: Terminate client connections before Workspace is destroyed
ClosedPublic

Authored by zzag on Jul 30 2019, 8:52 AM.

Details

Summary

When ShellClient tears down, it needs to access RuleBook in order to
discard temporary rules. The problem is that WaylandServer outlives
Workspace and therefore so does ShellClient.

We can't guard against the case when RuleBook::self() is nullptr as it
is vital to discard temporary rules.

This change adjusts termination sequence so all shell clients are
destroyed before Workspace(and thus RuleBook) is gone.

ASAN output:

==19922==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000142060 at pc 0x7fbc0fb878bb bp 0x7ffd7d464520 sp 0x7ffd7d464518
READ of size 8 at 0x606000142060 thread T0
    #0 0x7fbc0fb878ba in QList<KWin::Rules*>::detach() /usr/include/qt5/QtCore/qlist.h:172
    #1 0x7fbc0fb8538d in QList<KWin::Rules*>::begin() /usr/include/qt5/QtCore/qlist.h:324
    #2 0x7fbc0fb808b6 in KWin::RuleBook::discardUsed(KWin::AbstractClient*, bool) /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/rules.cpp:1144
    #3 0x7fbc0fe36e32 in KWin::ShellClient::destroyClient() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/shell_client.cpp:435
    #4 0x7fbc0fe7a726 in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (KWin::ShellClient::*)()>::call(void (KWin::ShellClient::*)(), KWin::ShellClient*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:152
    #5 0x7fbc0fe784c3 in void QtPrivate::FunctionPointer<void (KWin::ShellClient::*)()>::call<QtPrivate::List<>, void>(void (KWin::ShellClient::*)(), KWin::ShellClient*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:185
    #6 0x7fbc0fe74de9 in QtPrivate::QSlotObject<void (KWin::ShellClient::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/home/jenkins/install-prefix/lib64/libkwin.so.5+0x1677de9)
    #7 0x7fbc04f27357 in QMetaObject::activate(QObject*, int, int, void**) (/usr/lib64/libQt5Core.so.5+0x2b3357)
    #8 0x7fbc074e1970 in KWayland::Server::Resource::unbound() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.12/kwayland/build/src/server/KF5WaylandServer_autogen/EWIEGA46WW/moc_resource.cpp:142
    #9 0x7fbc0766b4b4 in KWayland::Server::Resource::Private::unbind(wl_resource*) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.12/kwayland/src/server/resource.cpp:68
    #10 0x7fbc00bdc2ae  (/usr/lib64/libwayland-server.so.0+0x92ae)
    #11 0x7fbc00bdc32f in wl_resource_destroy (/usr/lib64/libwayland-server.so.0+0x932f)
    #12 0x7fbc0766b53f in KWayland::Server::Resource::Private::resourceDestroyedCallback(wl_client*, wl_resource*) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.12/kwayland/src/server/resource.cpp:76
    #13 0x7fbbff481d8c  (/usr/lib64/libffi.so.7+0x6d8c)
    #14 0x7fbbff481179  (/usr/lib64/libffi.so.7+0x6179)
    #15 0x7fbc00bdfa5f  (/usr/lib64/libwayland-server.so.0+0xca5f)
    #16 0x7fbc00bdc6d1  (/usr/lib64/libwayland-server.so.0+0x96d1)
    #17 0x7fbc00bddc71 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xac71)
    #18 0x7fbc07541e50 in KWayland::Server::Display::Private::dispatch() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.12/kwayland/src/server/display.cpp:148
    #19 0x7fbc075432de in KWayland::Server::Display::dispatchEvents(int) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.12/kwayland/src/server/display.cpp:220
    #20 0x7fbc0fe864ca in KWin::WaylandServer::dispatch() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/wayland_server.cpp:616
    #21 0x451ce0 in KWin::WaylandTestApplication::~WaylandTestApplication() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/autotests/integration/kwin_wayland_test.cpp:91
    #22 0x42faa1 in main /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/autotests/integration/globalshortcuts_test.cpp:381
    #23 0x7fbc04796bca in __libc_start_main (/lib64/libc.so.6+0x26bca)
    #24 0x413ea9 in _start (/home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.12/build/bin/testGlobalShortcuts+0x413ea9)

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zzag created this revision.Jul 30 2019, 8:52 AM
Restricted Application added a project: KWin. · View Herald TranscriptJul 30 2019, 8:52 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jul 30 2019, 8:52 AM
zzag added a comment.Jul 30 2019, 8:53 AM

If there's some alternative solution to this nasty problem, I'd love to discuss it.

zzag updated this revision to Diff 62781.Jul 30 2019, 8:55 AM

I forgot to delete a method (shame!).

zzag edited the summary of this revision. (Show Details)Jul 30 2019, 9:04 AM
davidedmundson accepted this revision.Jul 31 2019, 5:04 PM
This revision is now accepted and ready to land.Jul 31 2019, 5:04 PM
This revision was automatically updated to reflect the committed changes.