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

[Chris Halse Rogers]

Wayland: Close the Mir session on client quit.

Fixes phantom sessions hanging around after client quit.

=== modified file 'debian/control'
--- debian/control	2017-09-13 04:15:28 +0000
+++ debian/control	2017-09-26 15:46:44 +0000
@@ -47,6 +47,7 @@
                capnproto,
                libepoxy-dev,
                python3-pil:native,
+               linux-libc-dev,
 Standards-Version: 3.9.4
 Homepage: https://launchpad.net/mir
 # If you aren't a member of ~mir-team but need to upload packaging changes,
 
=== modified file 'src/protocol/wrapper_generator.cpp'
--- src/protocol/wrapper_generator.cpp	2017-09-07 00:32:52 +0000
+++ src/protocol/wrapper_generator.cpp	2017-09-26 15:46:44 +0000
@@ -373,6 +373,16 @@
 
         if (!methods.empty())
         {
+            if (!is_global)
+            {
+                emit_indented_lines(out, "    ", {
+                    {"static void resource_destroyed_thunk(wl_resource* us)"},
+                    {"{"},
+                    {"    delete static_cast<", generated_name, "*>(wl_resource_get_user_data(us));"},
+                    {"}"}
+                });
+            }
+
             emit_indented_lines(out, "    ", {
                 { "static struct ", wl_name, "_interface const vtable;" }
             });
@@ -427,7 +437,7 @@
         if (has_vtable)
         {
             emit_indented_lines(out, indent + "    ",
-                {{ "wl_resource_set_implementation(resource, &vtable, this, nullptr);" }});
+                {{ "wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);" }});
         }
         emit_indented_lines(out, indent, {
             { "}" }
 
=== modified file 'src/server/frontend/wayland/core_generated_interfaces.h'
--- src/server/frontend/wayland/core_generated_interfaces.h	2017-09-07 00:33:43 +0000
+++ src/server/frontend/wayland/core_generated_interfaces.h	2017-09-26 15:46:44 +0000
@@ -108,7 +108,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~ShmPool() = default;
 
@@ -138,6 +138,10 @@
         me->resize(size);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<ShmPool*>(wl_resource_get_user_data(us));
+    }
     static struct wl_shm_pool_interface const vtable;
 };
 
@@ -207,7 +211,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Buffer() = default;
 
@@ -223,6 +227,10 @@
         me->destroy();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Buffer*>(wl_resource_get_user_data(us));
+    }
     static struct wl_buffer_interface const vtable;
 };
 
@@ -243,7 +251,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~DataOffer() = default;
 
@@ -293,6 +301,10 @@
         me->set_actions(dnd_actions, preferred_action);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<DataOffer*>(wl_resource_get_user_data(us));
+    }
     static struct wl_data_offer_interface const vtable;
 };
 
@@ -317,7 +329,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~DataSource() = default;
 
@@ -347,6 +359,10 @@
         me->set_actions(dnd_actions);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<DataSource*>(wl_resource_get_user_data(us));
+    }
     static struct wl_data_source_interface const vtable;
 };
 
@@ -369,7 +385,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~DataDevice() = default;
 
@@ -414,6 +430,10 @@
         me->release();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<DataDevice*>(wl_resource_get_user_data(us));
+    }
     static struct wl_data_device_interface const vtable;
 };
 
@@ -536,7 +556,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~ShellSurface() = default;
 
@@ -625,6 +645,10 @@
         me->set_class(class_);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<ShellSurface*>(wl_resource_get_user_data(us));
+    }
     static struct wl_shell_surface_interface const vtable;
 };
 
@@ -654,7 +678,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Surface() = default;
 
@@ -748,6 +772,10 @@
         me->damage_buffer(x, y, width, height);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Surface*>(wl_resource_get_user_data(us));
+    }
     static struct wl_surface_interface const vtable;
 };
 
@@ -847,7 +875,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Pointer() = default;
 
@@ -875,6 +903,10 @@
         me->release();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Pointer*>(wl_resource_get_user_data(us));
+    }
     static struct wl_pointer_interface const vtable;
 };
 
@@ -896,7 +928,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Keyboard() = default;
 
@@ -912,6 +944,10 @@
         me->release();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Keyboard*>(wl_resource_get_user_data(us));
+    }
     static struct wl_keyboard_interface const vtable;
 };
 
@@ -932,7 +968,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Touch() = default;
 
@@ -948,6 +984,10 @@
         me->release();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Touch*>(wl_resource_get_user_data(us));
+    }
     static struct wl_touch_interface const vtable;
 };
 
@@ -1014,7 +1054,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Region() = default;
 
@@ -1044,6 +1084,10 @@
         me->subtract(x, y, width, height);
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Region*>(wl_resource_get_user_data(us));
+    }
     static struct wl_region_interface const vtable;
 };
 
@@ -1120,7 +1164,7 @@
             wl_resource_post_no_memory(parent);
             BOOST_THROW_EXCEPTION((std::bad_alloc{}));
         }
-        wl_resource_set_implementation(resource, &vtable, this, nullptr);
+        wl_resource_set_implementation(resource, &vtable, this, &resource_destroyed_thunk);
     }
     virtual ~Subsurface() = default;
 
@@ -1171,6 +1215,10 @@
         me->set_desync();
     }
 
+    static void resource_destroyed_thunk(wl_resource* us)
+    {
+        delete static_cast<Subsurface*>(wl_resource_get_user_data(us));
+    }
     static struct wl_subsurface_interface const vtable;
 };
 
 
=== modified file 'src/server/frontend/wayland/wayland_connector.cpp'
--- src/server/frontend/wayland/wayland_connector.cpp	2017-09-25 18:12:01 +0000
+++ src/server/frontend/wayland/wayland_connector.cpp	2017-09-26 15:46:44 +0000
@@ -41,6 +41,8 @@
 
 #include "mir/executor.h"
 
+#include "mir/client/event.h"
+
 #include <system_error>
 #include <sys/eventfd.h>
 #include <wayland-server-core.h>
@@ -51,6 +53,7 @@
 #include <functional>
 #include <type_traits>
 
+#include <linux/input.h>
 #include <algorithm>
 #include <iostream>
 #include <mir/log.h>
@@ -69,6 +72,7 @@
 namespace mc = mir::compositor;
 namespace ms = mir::scene;
 namespace geom = mir::geometry;
+namespace mcl = mir::client;
 
 namespace mir
 {
@@ -154,8 +158,29 @@
 
 struct ClientPrivate
 {
+    ClientPrivate(std::shared_ptr<mf::Session> const& session, mf::Shell& shell)
+        : session{session},
+          shell{&shell}
+    {
+    }
+
+    ~ClientPrivate()
+    {
+        shell->close_session(session);
+        /*
+         * This ensures that further calls to
+         * wl_client_get_destroy_listener(client, &cleanup_private)
+         * - and hence session_for_client(client) - return nullptr.
+         */
+        wl_list_remove(&destroy_listener.link);
+    }
+
     wl_listener destroy_listener;
-    std::shared_ptr<mf::Session> session;
+    std::shared_ptr<mf::Session> const session;
+    /*
+     * This shell is owned by the ClientSessionConstructor, which outlives all clients.
+     */
+    mf::Shell* const shell;
 };
 
 static_assert(
@@ -178,9 +203,10 @@
 {
     auto listener = wl_client_get_destroy_listener(client, &cleanup_private);
 
-    assert(listener && "Client session requested for malformed client");
+    if (listener)
+        return private_from_listener(listener)->session;
 
-    return private_from_listener(listener)->session;
+    return nullptr;
 }
 
 struct ClientSessionConstructor
@@ -216,9 +242,8 @@
         "",
         std::make_shared<WaylandEventSink>([](auto){}));
 
-    auto client_context = new ClientPrivate;
+    auto client_context = new ClientPrivate{session, *construction_context->shell};
     client_context->destroy_listener.notify = &cleanup_private;
-    client_context->session = session;
     wl_client_add_destroy_listener(client, &client_context->destroy_listener);
 }
 
@@ -291,6 +316,19 @@
 
     return buffer;
 }
+
+template<typename Callable>
+auto run_unless(std::shared_ptr<bool> const& condition, Callable&& callable)
+{
+    return
+        [callable = std::move(callable), condition]()
+        {
+            if (*condition)
+                return;
+
+            callable();
+        };
+}
 }
 
 class WlShmBuffer :
@@ -507,7 +545,8 @@
           allocator{allocator},
           executor{executor},
           pending_buffer{nullptr},
-          pending_frames{std::make_shared<std::vector<wl_resource*>>()}
+          pending_frames{std::make_shared<std::vector<wl_resource*>>()},
+          destroyed{std::make_shared<bool>(false)}
     {
         auto session = session_for_client(client);
         mg::BufferProperties const props{
@@ -523,6 +562,13 @@
         stream->allow_framedropping(true);
     }
 
+    ~WlSurface()
+    {
+        *destroyed = true;
+        if (auto session = session_for_client(client))
+            session->destroy_buffer_stream(stream_id);
+    }
+
     void set_resize_handler(std::function<void(geom::Size)> const& handler)
     {
         resize_handler = handler;
@@ -544,6 +590,7 @@
 
     wl_resource* pending_buffer;
     std::shared_ptr<std::vector<wl_resource*>> const pending_frames;
+    std::shared_ptr<bool> const destroyed;
 
     void destroy();
     void attach(std::experimental::optional<wl_resource*> const& buffer, int32_t x, int32_t y);
@@ -559,7 +606,7 @@
 
 void WlSurface::destroy()
 {
-    delete this;
+    wl_resource_destroy(resource);
 }
 
 void WlSurface::attach(std::experimental::optional<wl_resource*> const& buffer, int32_t x, int32_t y)
@@ -616,9 +663,10 @@
         std::shared_ptr<mg::Buffer> mir_buffer;
         auto shm_buffer = wl_shm_buffer_get(pending_buffer);
         auto send_frame_notifications =
-            [executor = executor, frames = pending_frames]()
+            [executor = executor, frames = pending_frames, destroyed = destroyed]()
             {
-                executor->spawn(
+                executor->spawn(run_unless(
+                    destroyed,
                     [frames]()
                     {
                         /*
@@ -635,7 +683,7 @@
                             wl_resource_destroy(frame);
                         }
                         frames->clear();
-                    });
+                    }));
             };
 
         if (shm_buffer)
@@ -748,16 +796,29 @@
         wl_client* client,
         wl_resource* parent,
         uint32_t id,
+        std::function<void(WlKeyboard*)> const& on_destroy,
         std::shared_ptr<mir::Executor> const& executor)
         : Keyboard(client, parent, id),
-          executor{executor}
-    {
+          executor{executor},
+          on_destroy{on_destroy},
+          destroyed{std::make_shared<bool>(false)}
+    {
+    }
+
+    ~WlKeyboard()
+    {
+        on_destroy(this);
+        *destroyed = true;
     }
 
     void handle_event(MirInputEvent const* event, wl_resource* /*target*/)
     {
-        executor->spawn(
-            [ev = mir_event_ref(mir_input_event_get_event(event)), this] ()
+        executor->spawn(run_unless(
+            destroyed,
+            [
+                ev = mcl::Event{mir_event_ref(mir_input_event_get_event(event))},
+                this
+            ] ()
             {
                 int const serial = wl_display_next_serial(wl_client_get_display(client));
                 auto event = mir_event_get_input_event(ev);
@@ -782,18 +843,22 @@
                     default:
                         break;
                 }
-                mir_event_unref(ev);
-            });
+            }));
     }
 
     void handle_event(MirWindowEvent const* event, wl_resource* target)
     {
         if (mir_window_event_get_attribute(event) == mir_window_attrib_focus)
         {
-            executor->spawn(
-               [resource = resource, serial = wl_display_next_serial(wl_client_get_display(client)),
-                target = target, focussed = mir_window_event_get_attribute_value(event)]()
+            executor->spawn(run_unless(
+                destroyed,
+                [
+                    target = target,
+                    focussed = mir_window_event_get_attribute_value(event),
+                    this
+                ]()
                 {
+                    auto const serial = wl_display_next_serial(wl_client_get_display(client));
                     if (focussed)
                     {
                         wl_array key_state;
@@ -805,18 +870,21 @@
                     {
                         wl_keyboard_send_leave(resource, serial, target);
                     }
-                });
+                }));
         }
     }
 
 private:
     std::shared_ptr<mir::Executor> const executor;
+    std::function<void(WlKeyboard*)> on_destroy;
+    std::shared_ptr<bool> const destroyed;
 
     void release() override;
 };
 
 void WlKeyboard::release()
 {
+    wl_resource_destroy(resource);
 }
 
 class WlPointer : public wayland::Pointer
@@ -827,17 +895,27 @@
         wl_client* client,
         wl_resource* parent,
         uint32_t id,
+        std::function<void(WlPointer*)> const& on_destroy,
         std::shared_ptr<mir::Executor> const& executor)
         : Pointer(client, parent, id),
           display{wl_client_get_display(client)},
-          executor{executor}
-    {
+          executor{executor},
+          on_destroy{on_destroy},
+          destroyed{std::make_shared<bool>(false)}
+    {
+    }
+
+    ~WlPointer()
+    {
+        on_destroy(this);
+        *destroyed = true;
     }
 
     void handle_event(MirInputEvent const* event, wl_resource* target)
     {
-        executor->spawn(
-            [ev = mir_event_ref(mir_input_event_get_event(event)), target, this]()
+        executor->spawn(run_unless(
+            destroyed,
+            [ev = mcl::Event{mir_input_event_get_event(event)}, target, this]()
             {
                 auto const serial = wl_display_next_serial(display);
                 auto const event = mir_event_get_input_event(ev);
@@ -851,25 +929,27 @@
                         auto const current_set  = mir_pointer_event_buttons(pointer_event);
                         auto const current_time = mir_input_event_get_event_time(event) / 1000;
 
-                        auto button = 272;  // No, I have *no* idea why GTK expects 272 to be the primary button.
-                        // NB button is incremented in the loop and the order mir_pointer_button_XXX matters
-                        for (auto mir_button :
-                            {mir_pointer_button_primary,    // 272
-                             mir_pointer_button_tertiary,   // 273
-                             mir_pointer_button_secondary,  // 274
-                             mir_pointer_button_back,       // 275
-                             mir_pointer_button_forward})   // 276
+                        std::unordered_map<MirPointerButton, int> const button_mappings = {
+                            {mir_pointer_button_primary, BTN_LEFT},
+                            {mir_pointer_button_secondary, BTN_RIGHT},
+                            {mir_pointer_button_tertiary, BTN_MIDDLE},
+                            {mir_pointer_button_back, BTN_BACK},
+                            {mir_pointer_button_forward, BTN_FORWARD},
+                            {mir_pointer_button_side, BTN_SIDE},
+                            {mir_pointer_button_task, BTN_TASK},
+                            {mir_pointer_button_extra, BTN_EXTRA}
+                        };
+
+                        for (auto mapping : button_mappings)
                         {
-                            if (mir_button & (current_set ^ last_set))
+                            if (mapping.first & (current_set ^ last_set))
                             {
-                                auto const action = (mir_button & current_set) ?
+                                auto const action = (mapping.first & current_set) ?
                                                     WL_POINTER_BUTTON_STATE_PRESSED :
                                                     WL_POINTER_BUTTON_STATE_RELEASED;
 
-                                wl_pointer_send_button(resource, serial, current_time, button, action);
+                                wl_pointer_send_button(resource, serial, current_time, mapping.second, action);
                             }
-
-                            ++button;
                         }
 
                         last_set = current_set;
@@ -934,7 +1014,7 @@
                     case mir_pointer_actions:
                         break;
                 }
-            });
+            }));
     }
 
     // Pointer interface
@@ -942,6 +1022,9 @@
     wl_display* const display;
     std::shared_ptr<mir::Executor> const executor;
 
+    std::function<void(WlPointer*)> on_destroy;
+    std::shared_ptr<bool> const destroyed;
+
     MirPointerButtons last_set{0};
     float last_x, last_y, last_vscroll, last_hscroll;
 
@@ -959,7 +1042,7 @@
 
 void WlPointer::release()
 {
-    delete this;
+    wl_resource_destroy(resource);
 }
 
 class WlTouch : public wayland::Touch
@@ -969,9 +1052,18 @@
         wl_client* client,
         wl_resource* parent,
         uint32_t id,
-        std::shared_ptr<mir::Executor> const& /*executor*/)
-        : Touch(client, parent, id)
-    {
+        std::function<void(WlTouch*)> const& on_destroy,
+        std::shared_ptr<mir::Executor> const& executor)
+        : Touch(client, parent, id),
+          executor{executor},
+          on_destroy{on_destroy}
+    {
+    }
+
+    ~WlTouch()
+    {
+        on_destroy(this);
+        *destroyed = true;
     }
 
     void handle_event(MirInputEvent const* /*event*/, wl_resource* /*target*/)
@@ -980,11 +1072,16 @@
 
     // Touch interface
 private:
+    std::shared_ptr<mir::Executor> const executor;
+    std::function<void(WlTouch*)> on_destroy;
+    std::shared_ptr<bool> const destroyed;
+
     void release() override;
 };
 
 void WlTouch::release()
 {
+    wl_resource_destroy(resource);
 }
 
 template<class InputInterface>
@@ -997,25 +1094,27 @@
     InputCtx(InputCtx const&) = delete;
     InputCtx& operator=(InputCtx const&) = delete;
 
-    void register_listener(std::shared_ptr<InputInterface> const& listener)
+    void register_listener(InputInterface* listener)
     {
+        std::lock_guard<std::mutex> lock{mutex};
         listeners.push_back(listener);
     }
 
     void unregister_listener(InputInterface const* listener)
     {
-        std::remove_if(
-            listeners.begin(),
-            listeners.end(),
-            [listener](auto candidate)
-            {
-                return candidate.get() == listener;
-            });
+        std::lock_guard<std::mutex> lock{mutex};
+        listeners.erase(
+            std::remove(
+                listeners.begin(),
+                listeners.end(),
+                listener),
+            listeners.end());
     }
 
     void handle_event(MirInputEvent const* event, wl_resource* target) const
     {
-        for (auto& listener : listeners)
+        std::lock_guard<std::mutex> lock{mutex};
+        for (auto listener : listeners)
         {
             listener->handle_event(event, target);
         }
@@ -1023,6 +1122,7 @@
 
     void handle_event(MirWindowEvent const* event, wl_resource* target) const
     {
+        std::lock_guard<std::mutex> lock{mutex};
         for (auto& listener : listeners)
         {
             listener->handle_event(event, target);
@@ -1030,7 +1130,8 @@
     }
 
 private:
-    std::vector<std::shared_ptr<InputInterface>> listeners;
+    std::mutex mutable mutex;
+    std::vector<InputInterface*> listeners;
 };
 
 class WlSeat
@@ -1100,32 +1201,49 @@
     static void get_pointer(wl_client* client, wl_resource* resource, uint32_t id)
     {
         auto me = reinterpret_cast<WlSeat*>(wl_resource_get_user_data(resource));
-        me->pointer[client].register_listener(
-            std::make_shared<WlPointer>(
+        auto& input_ctx = me->pointer[client];
+        input_ctx.register_listener(
+            new WlPointer{
                 client,
                 resource,
                 id,
-                me->executor));
+                [&input_ctx](WlPointer* listener)
+                {
+                    input_ctx.unregister_listener(listener);
+                },
+                me->executor});
     }
     static void get_keyboard(wl_client* client, wl_resource* resource, uint32_t id)
     {
         auto me = reinterpret_cast<WlSeat*>(wl_resource_get_user_data(resource));
-        me->keyboard[client].register_listener(
-            std::make_shared<WlKeyboard>(
+        auto& input_ctx = me->keyboard[client];
+
+        input_ctx.register_listener(
+            new WlKeyboard{
                 client,
                 resource,
                 id,
-                me->executor));
+                [&input_ctx](WlKeyboard* listener)
+                {
+                    input_ctx.unregister_listener(listener);
+                },
+                me->executor});
     }
     static void get_touch(wl_client* client, wl_resource* resource, uint32_t id)
     {
         auto me = reinterpret_cast<WlSeat*>(wl_resource_get_user_data(resource));
-        me->touch[client].register_listener(
-            std::make_shared<WlTouch>(
+        auto& input_ctx = me->touch[client];
+
+        input_ctx.register_listener(
+            new WlTouch{
                 client,
                 resource,
                 id,
-                me->executor));
+                [&input_ctx](WlTouch* listener)
+                {
+                    input_ctx.unregister_listener(listener);
+                },
+                me->executor});
     }
     static void release(struct wl_client* /*client*/, struct wl_resource* /*resource*/) {}
 
@@ -1468,15 +1586,15 @@
                 hide_spec.state = mir_window_state_hidden;
                 shell->modify_surface(session, id, hide_spec);
             });
-
-        auto shim = new DestructionShim{session, shell, surface_id};
-        shim->destruction_listener.notify = &cleanup_surface;
-        wl_resource_add_destroy_listener(
-            resource,
-            &shim->destruction_listener);
-    }
-
-    ~WlShellSurface() override = default;
+    }
+
+    ~WlShellSurface() override
+    {
+        if (auto session = session_for_client(client))
+        {
+            shell->destroy_surface(session, surface_id);
+        }
+    }
 protected:
     void pong(uint32_t /*serial*/) override
     {
@@ -1516,7 +1634,6 @@
 
         auto const session = session_for_client(client);
         shell->modify_surface(session, surface_id, mods);
-
     }
 
     void set_popup(
@@ -1549,38 +1666,6 @@
     {
     }
 private:
-    struct DestructionShim
-    {
-        DestructionShim(
-            std::shared_ptr<mf::Session> const& session,
-            std::shared_ptr<mf::Shell> const& shell,
-            mf::SurfaceId id)
-            : session{session},
-              shell{shell},
-              surface_id{id}
-        {
-        }
-
-        std::shared_ptr<mf::Session> const session;
-        std::shared_ptr<mf::Shell> const shell;
-        mf::SurfaceId const surface_id;
-        wl_listener destruction_listener;
-    };
-
-    static_assert(
-        std::is_standard_layout<DestructionShim>::value,
-        "DestructionShim must be Standard Layout for wl_container_of to be defined behaviour");
-
-    static void cleanup_surface(wl_listener* listener, void*)
-    {
-        DestructionShim* shim;
-        shim = wl_container_of(listener, shim, destruction_listener);
-
-        shim->shell->destroy_surface(shim->session, shim->surface_id);
-
-        delete shim;
-    }
-
     std::shared_ptr<mf::Shell> const shell;
     mf::SurfaceId surface_id;
 };