Mir

Merge lp:~raof/mir/wayland-clients-close-their-sessions-on-quit into lp:mir

Proposed by Chris Halse Rogers on 2017-09-26
Status: Merged
Merged at revision: 4270
Proposed branch: lp:~raof/mir/wayland-clients-close-their-sessions-on-quit
Merge into: lp:mir
Prerequisite: lp:~raof/mir/wayland-input-fixes
Diff against target: 224 lines (+62/-64)
1 file modified
src/server/frontend/wayland/wayland_connector.cpp (+62/-64)
To merge this branch: bzr merge lp:~raof/mir/wayland-clients-close-their-sessions-on-quit
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Needs Fixing on 2017-09-26
Alan Griffiths 2017-09-26 Approve on 2017-09-26
Review via email: mp+331358@code.launchpad.net

Commit message

Wayland: Close the Mir session on client quit.

Fixes phantom sessions hanging around after client quit.

Fixes: https://bugs.launchpad.net/mir/+bug/1719586

To post a comment you must log in.
Alan Griffiths (alan-griffiths) wrote :

Looks sensible and seems to work.

review: Approve
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1443/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/5064/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1578/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5300
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5288
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5288
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5288
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/5108/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/5108/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/5108/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/5108/console
    ABORTED: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/5108/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/5108/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/5108/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/5108
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/5108/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/server/frontend/wayland/wayland_connector.cpp'
--- src/server/frontend/wayland/wayland_connector.cpp 2017-09-26 15:39:09 +0000
+++ src/server/frontend/wayland/wayland_connector.cpp 2017-09-26 15:39:09 +0000
@@ -158,8 +158,29 @@
158158
159struct ClientPrivate159struct ClientPrivate
160{160{
161 ClientPrivate(std::shared_ptr<mf::Session> const& session, mf::Shell& shell)
162 : session{session},
163 shell{&shell}
164 {
165 }
166
167 ~ClientPrivate()
168 {
169 shell->close_session(session);
170 /*
171 * This ensures that further calls to
172 * wl_client_get_destroy_listener(client, &cleanup_private)
173 * - and hence session_for_client(client) - return nullptr.
174 */
175 wl_list_remove(&destroy_listener.link);
176 }
177
161 wl_listener destroy_listener;178 wl_listener destroy_listener;
162 std::shared_ptr<mf::Session> session;179 std::shared_ptr<mf::Session> const session;
180 /*
181 * This shell is owned by the ClientSessionConstructor, which outlives all clients.
182 */
183 mf::Shell* const shell;
163};184};
164185
165static_assert(186static_assert(
@@ -182,9 +203,10 @@
182{203{
183 auto listener = wl_client_get_destroy_listener(client, &cleanup_private);204 auto listener = wl_client_get_destroy_listener(client, &cleanup_private);
184205
185 assert(listener && "Client session requested for malformed client");206 if (listener)
207 return private_from_listener(listener)->session;
186208
187 return private_from_listener(listener)->session;209 return nullptr;
188}210}
189211
190struct ClientSessionConstructor212struct ClientSessionConstructor
@@ -220,9 +242,8 @@
220 "",242 "",
221 std::make_shared<WaylandEventSink>([](auto){}));243 std::make_shared<WaylandEventSink>([](auto){}));
222244
223 auto client_context = new ClientPrivate;245 auto client_context = new ClientPrivate{session, *construction_context->shell};
224 client_context->destroy_listener.notify = &cleanup_private;246 client_context->destroy_listener.notify = &cleanup_private;
225 client_context->session = session;
226 wl_client_add_destroy_listener(client, &client_context->destroy_listener);247 wl_client_add_destroy_listener(client, &client_context->destroy_listener);
227}248}
228249
@@ -295,6 +316,19 @@
295316
296 return buffer;317 return buffer;
297}318}
319
320template<typename Callable>
321auto run_unless(std::shared_ptr<bool> const& condition, Callable&& callable)
322{
323 return
324 [callable = std::move(callable), condition]()
325 {
326 if (*condition)
327 return;
328
329 callable();
330 };
331}
298}332}
299333
300class WlShmBuffer :334class WlShmBuffer :
@@ -511,7 +545,8 @@
511 allocator{allocator},545 allocator{allocator},
512 executor{executor},546 executor{executor},
513 pending_buffer{nullptr},547 pending_buffer{nullptr},
514 pending_frames{std::make_shared<std::vector<wl_resource*>>()}548 pending_frames{std::make_shared<std::vector<wl_resource*>>()},
549 destroyed{std::make_shared<bool>(false)}
515 {550 {
516 auto session = session_for_client(client);551 auto session = session_for_client(client);
517 mg::BufferProperties const props{552 mg::BufferProperties const props{
@@ -527,6 +562,13 @@
527 stream->allow_framedropping(true);562 stream->allow_framedropping(true);
528 }563 }
529564
565 ~WlSurface()
566 {
567 *destroyed = true;
568 if (auto session = session_for_client(client))
569 session->destroy_buffer_stream(stream_id);
570 }
571
530 void set_resize_handler(std::function<void(geom::Size)> const& handler)572 void set_resize_handler(std::function<void(geom::Size)> const& handler)
531 {573 {
532 resize_handler = handler;574 resize_handler = handler;
@@ -548,6 +590,7 @@
548590
549 wl_resource* pending_buffer;591 wl_resource* pending_buffer;
550 std::shared_ptr<std::vector<wl_resource*>> const pending_frames;592 std::shared_ptr<std::vector<wl_resource*>> const pending_frames;
593 std::shared_ptr<bool> const destroyed;
551594
552 void destroy();595 void destroy();
553 void attach(std::experimental::optional<wl_resource*> const& buffer, int32_t x, int32_t y);596 void attach(std::experimental::optional<wl_resource*> const& buffer, int32_t x, int32_t y);
@@ -620,9 +663,10 @@
620 std::shared_ptr<mg::Buffer> mir_buffer;663 std::shared_ptr<mg::Buffer> mir_buffer;
621 auto shm_buffer = wl_shm_buffer_get(pending_buffer);664 auto shm_buffer = wl_shm_buffer_get(pending_buffer);
622 auto send_frame_notifications =665 auto send_frame_notifications =
623 [executor = executor, frames = pending_frames]()666 [executor = executor, frames = pending_frames, destroyed = destroyed]()
624 {667 {
625 executor->spawn(668 executor->spawn(run_unless(
669 destroyed,
626 [frames]()670 [frames]()
627 {671 {
628 /*672 /*
@@ -639,7 +683,7 @@
639 wl_resource_destroy(frame);683 wl_resource_destroy(frame);
640 }684 }
641 frames->clear();685 frames->clear();
642 });686 }));
643 };687 };
644688
645 if (shm_buffer)689 if (shm_buffer)
@@ -745,19 +789,6 @@
745class WlPointer;789class WlPointer;
746class WlTouch;790class WlTouch;
747791
748template<typename Callable>
749auto run_unless(std::shared_ptr<bool> const& condition, Callable&& callable)
750{
751 return
752 [callable = std::move(callable), condition]()
753 {
754 if (*condition)
755 return;
756
757 callable();
758 };
759}
760
761class WlKeyboard : public wayland::Keyboard792class WlKeyboard : public wayland::Keyboard
762{793{
763public:794public:
@@ -1555,15 +1586,15 @@
1555 hide_spec.state = mir_window_state_hidden;1586 hide_spec.state = mir_window_state_hidden;
1556 shell->modify_surface(session, id, hide_spec);1587 shell->modify_surface(session, id, hide_spec);
1557 });1588 });
15581589 }
1559 auto shim = new DestructionShim{session, shell, surface_id};1590
1560 shim->destruction_listener.notify = &cleanup_surface;1591 ~WlShellSurface() override
1561 wl_resource_add_destroy_listener(1592 {
1562 resource,1593 if (auto session = session_for_client(client))
1563 &shim->destruction_listener);1594 {
1564 }1595 shell->destroy_surface(session, surface_id);
15651596 }
1566 ~WlShellSurface() override = default;1597 }
1567protected:1598protected:
1568 void pong(uint32_t /*serial*/) override1599 void pong(uint32_t /*serial*/) override
1569 {1600 {
@@ -1603,7 +1634,6 @@
16031634
1604 auto const session = session_for_client(client);1635 auto const session = session_for_client(client);
1605 shell->modify_surface(session, surface_id, mods);1636 shell->modify_surface(session, surface_id, mods);
1606
1607 }1637 }
16081638
1609 void set_popup(1639 void set_popup(
@@ -1636,38 +1666,6 @@
1636 {1666 {
1637 }1667 }
1638private:1668private:
1639 struct DestructionShim
1640 {
1641 DestructionShim(
1642 std::shared_ptr<mf::Session> const& session,
1643 std::shared_ptr<mf::Shell> const& shell,
1644 mf::SurfaceId id)
1645 : session{session},
1646 shell{shell},
1647 surface_id{id}
1648 {
1649 }
1650
1651 std::shared_ptr<mf::Session> const session;
1652 std::shared_ptr<mf::Shell> const shell;
1653 mf::SurfaceId const surface_id;
1654 wl_listener destruction_listener;
1655 };
1656
1657 static_assert(
1658 std::is_standard_layout<DestructionShim>::value,
1659 "DestructionShim must be Standard Layout for wl_container_of to be defined behaviour");
1660
1661 static void cleanup_surface(wl_listener* listener, void*)
1662 {
1663 DestructionShim* shim;
1664 shim = wl_container_of(listener, shim, destruction_listener);
1665
1666 shim->shell->destroy_surface(shim->session, shim->surface_id);
1667
1668 delete shim;
1669 }
1670
1671 std::shared_ptr<mf::Shell> const shell;1669 std::shared_ptr<mf::Shell> const shell;
1672 mf::SurfaceId surface_id;1670 mf::SurfaceId surface_id;
1673};1671};

Subscribers

People subscribed via source and target branches