Bind EventHandler lifetimes to owner

src/logid/backend/EventHandlerList.h

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2019-2023 PixlOne
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+#ifndef LOGID_EVENTHANDLERLIST_H
+#define LOGID_EVENTHANDLERLIST_H
+
+#include <memory>
+#include <mutex>
+#include <shared_mutex>
+#include <list>
+
+template <class T>
+class EventHandlerLock;
+
+template <class T>
+struct EventHandlerList {
+    typedef std::list<typename T::EventHandler> list_t;
+    typedef list_t::const_iterator iterator_t;
+
+    std::list<typename T::EventHandler> list;
+    mutable std::shared_mutex mutex;
+
+    void remove(iterator_t iterator) {
+        std::unique_lock lock(mutex);
+        list.erase(iterator);
+    }
+};
+
+template <class T>
+class EventHandlerLock {
+    typedef EventHandlerList<T> list_t;
+    typedef typename list_t::iterator_t iterator_t;
+
+    friend T;
+
+    std::weak_ptr<list_t> _list;
+    iterator_t _iterator;
+
+    EventHandlerLock(const std::shared_ptr<list_t>& list, iterator_t iterator) :
+                     _list (list), _iterator (iterator) {
+    }
+public:
+    EventHandlerLock() = default;
+
+    EventHandlerLock(const EventHandlerLock&) = delete;
+
+    EventHandlerLock(EventHandlerLock&& o) noexcept : _list (o._list), _iterator (o._iterator) {
+        o._list.reset();
+    }
+
+    EventHandlerLock& operator=(const EventHandlerLock&) = delete;
+
+    EventHandlerLock& operator=(EventHandlerLock&& o) noexcept {
+        this->_list = o._list;
+        this->_iterator = o._iterator;
+        o._list.reset();
+
+        return *this;
+    }
+
+    ~EventHandlerLock() {
+        if(auto list = _list.lock())
+            list->remove(_iterator);
+    }
+
+    [[nodiscard]] bool empty() const noexcept {
+        return _list.expired();
+    }
+};
+
+#endif //LOGID_EVENTHANDLERLIST_H

src/logid/backend/hidpp/Device.cpp

@@ -108,6 +108,8 @@ const std::tuple<uint8_t, uint8_t>& Device::version() const {
 }
 
 void Device::_init() {
+    _event_handlers = std::make_shared<EventHandlerList<Device>>();
+
     supported_reports = getSupportedReports(_raw_device->reportDescriptor());
     if (!supported_reports)
         throw InvalidDevice(InvalidDevice::NoHIDPPReport);
@@ -135,12 +137,7 @@ void Device::_init() {
             }
         } catch (hidpp10::Error& e) {
             // Ignore
-        } catch (std::exception& e) {
-            _raw_device->removeEventHandler(_raw_handler);
-            throw;
         }
-
-        _raw_device->removeEventHandler(_raw_handler);
     }
 
     _raw_handler = _raw_device->addEventHandler(
@@ -155,7 +152,6 @@ void Device::_init() {
                  this->handleEvent(_report);
              }});
 
-    try {
     try {
         hidpp20::Root root(this);
         _version = root.getVersion();
@@ -192,33 +188,20 @@ void Device::_init() {
             _name = _receiver->getDeviceName(_index);
         }
     }
-    } catch (std::exception& e) {
-        _raw_device->removeEventHandler(_raw_handler);
-        throw;
-    }
 }
 
-Device::~Device() {
+EventHandlerLock<Device> Device::addEventHandler(EventHandler handler) {
-    _raw_device->removeEventHandler(_raw_handler);
+    std::unique_lock lock(_event_handlers->mutex);
-}
+    _event_handlers->list.emplace_front(std::move(handler));
-
+    return {_event_handlers, _event_handlers->list.cbegin()};
-Device::EvHandlerId Device::addEventHandler(EventHandler handler) {
-    std::unique_lock lock(_event_handler_mutex);
-    _event_handlers.emplace_front(std::move(handler));
-    return _event_handlers.cbegin();
-}
-
-void Device::removeEventHandler(EvHandlerId id) {
-    std::unique_lock lock(_event_handler_mutex);
-    _event_handlers.erase(id);
 }
 
 void Device::handleEvent(Report& report) {
     if (responseReport(report))
         return;
 
-    std::shared_lock lock(_event_handler_mutex);
+    std::shared_lock lock(_event_handlers->mutex);
-    for (auto& handler: _event_handlers)
+    for (auto& handler: _event_handlers->list)
         if (handler.condition(report))
             handler.callback(report);
 }

src/logid/backend/hidpp/Device.h

@@ -28,6 +28,7 @@
 #include <backend/raw/RawDevice.h>
 #include <backend/hidpp/Report.h>
 #include <backend/hidpp/defs.h>
+#include <backend/EventHandlerList.h>
 
 namespace logid::backend::hidpp10 {
     // Need to define here for a constructor
@@ -36,15 +37,14 @@ namespace logid::backend::hidpp10 {
 
 namespace logid::backend::hidpp {
     struct DeviceConnectionEvent;
+
+    class Device {
+    public:
         struct EventHandler {
             std::function<bool(Report&)> condition;
             std::function<void(Report&)> callback;
         };
 
-    class Device {
-    public:
-        typedef std::list<EventHandler>::const_iterator EvHandlerId;
-
         class InvalidDevice : std::exception {
         public:
             enum Reason {
@@ -76,8 +76,6 @@ namespace logid::backend::hidpp {
         Device(const std::shared_ptr<hidpp10::Receiver>& receiver,
                DeviceIndex index, double timeout);
 
-        virtual ~Device();
-
         [[nodiscard]] const std::string& devicePath() const;
 
         [[nodiscard]] DeviceIndex deviceIndex() const;
@@ -88,9 +86,7 @@ namespace logid::backend::hidpp {
 
         [[nodiscard]] uint16_t pid() const;
 
-        EvHandlerId addEventHandler(EventHandler handler);
+        EventHandlerLock<Device> addEventHandler(EventHandler handler);
-
-        void removeEventHandler(EvHandlerId id);
 
         virtual Report sendReport(const Report& report);
 
@@ -117,7 +113,7 @@ namespace logid::backend::hidpp {
         void _init();
 
         std::shared_ptr<raw::RawDevice> _raw_device;
-        raw::RawDevice::EvHandlerId _raw_handler;
+        EventHandlerLock<raw::RawDevice> _raw_handler;
         std::shared_ptr<hidpp10::Receiver> _receiver;
         std::string _path;
         DeviceIndex _index;
@@ -133,9 +129,10 @@ namespace logid::backend::hidpp {
         std::optional<Response> _response;
         std::optional<uint8_t> _sent_sub_id{};
 
-        std::shared_mutex _event_handler_mutex;
+        std::shared_ptr<EventHandlerList<Device>> _event_handlers;
-        std::list<EventHandler> _event_handlers;
     };
+
+    typedef Device::EventHandler EventHandler;
 }
 
 #endif //LOGID_BACKEND_HIDPP_DEVICE_H

src/logid/backend/hidpp10/ReceiverMonitor.cpp

@@ -32,22 +32,8 @@ ReceiverMonitor::ReceiverMonitor(const std::string& path,
     _receiver->setNotifications(notification_flags);
 }
 
-ReceiverMonitor::~ReceiverMonitor() {
-    if (_pair_status_handler.has_value())
-        _receiver->removeEventHandler(_pair_status_handler.value());
-
-    if (_passkey_ev_handler.has_value())
-        _receiver->removeEventHandler(_passkey_ev_handler.value());
-
-    if (_discover_ev_handler.has_value())
-        _receiver->removeEventHandler(_discover_ev_handler.value());
-
-    if (_connect_ev_handler.has_value())
-        _receiver->rawDevice()->removeEventHandler(_connect_ev_handler.value());
-}
-
 void ReceiverMonitor::ready() {
-    if (!_connect_ev_handler.has_value()) {
+    if (_connect_ev_handler.empty()) {
         _connect_ev_handler = _receiver->rawDevice()->addEventHandler(
                 {[](const std::vector<uint8_t>& report) -> bool {
                     if (report[Offset::Type] == Report::Type::Short ||
@@ -86,7 +72,7 @@ void ReceiverMonitor::ready() {
                 });
     }
 
-    if (!_discover_ev_handler.has_value()) {
+    if (_discover_ev_handler.empty()) {
         _discover_ev_handler = _receiver->addEventHandler(
                 {[](const hidpp::Report& report) -> bool {
                     return (report.subId() == Receiver::DeviceDiscovered) &&
@@ -108,7 +94,7 @@ void ReceiverMonitor::ready() {
                 });
     }
 
-    if (!_passkey_ev_handler.has_value()) {
+    if (_passkey_ev_handler.empty()) {
         _passkey_ev_handler = _receiver->addEventHandler(
                 {[](const hidpp::Report& report) -> bool {
                     return report.subId() == Receiver::PasskeyRequest &&
@@ -126,7 +112,7 @@ void ReceiverMonitor::ready() {
                 });
     }
 
-    if (!_pair_status_handler.has_value()) {
+    if (_pair_status_handler.empty()) {
         _pair_status_handler = _receiver->addEventHandler(
                 {[](const hidpp::Report& report) -> bool {
                     return report.subId() == Receiver::DiscoveryStatus ||
@@ -166,7 +152,7 @@ void ReceiverMonitor::enumerate() {
 }
 
 void ReceiverMonitor::waitForDevice(hidpp::DeviceIndex index) {
-    auto handler_id = std::make_shared<raw::RawDevice::EvHandlerId>();
+    auto handler_id = std::make_shared<EventHandlerLock<raw::RawDevice>>();
 
     *handler_id = _receiver->rawDevice()->addEventHandler(
             {[index](const std::vector<uint8_t>& report) -> bool {
@@ -180,9 +166,8 @@ void ReceiverMonitor::waitForDevice(hidpp::DeviceIndex index) {
                  event.fromTimeoutCheck = true;
 
                  spawn_task([this, event, handler_id]() {
-                     assert(handler_id);
+                     *handler_id = {};
                      try {
-                         _receiver->rawDevice()->removeEventHandler(*handler_id);
                          addDevice(event);
                      } catch (std::exception& e) {
                          logPrintf(ERROR, "Failed to add device %d to receiver on %s: %s",

src/logid/backend/hidpp10/ReceiverMonitor.h

@@ -32,8 +32,6 @@ namespace logid::backend::hidpp10 {
                         const std::shared_ptr<raw::DeviceMonitor>& monitor,
                         double timeout);
 
-        virtual ~ReceiverMonitor();
-
         void enumerate();
 
     protected:
@@ -68,11 +66,11 @@ namespace logid::backend::hidpp10 {
         DeviceDiscoveryEvent _discovery_event;
         PairState _pair_state = NotPairing;
 
-        std::optional<raw::RawDevice::EvHandlerId> _connect_ev_handler;
+        EventHandlerLock<raw::RawDevice> _connect_ev_handler;
 
-        std::optional<hidpp::Device::EvHandlerId> _discover_ev_handler;
+        EventHandlerLock<hidpp::Device> _discover_ev_handler;
-        std::optional<hidpp::Device::EvHandlerId> _passkey_ev_handler;
+        EventHandlerLock<hidpp::Device> _passkey_ev_handler;
-        std::optional<hidpp::Device::EvHandlerId> _pair_status_handler;
+        EventHandlerLock<hidpp::Device> _pair_status_handler;
     };
 
 }

src/logid/backend/raw/RawDevice.cpp

@@ -74,7 +74,8 @@ std::string get_name(int fd) {
 RawDevice::RawDevice(std::string path, const std::shared_ptr<DeviceMonitor>& monitor) :
         _valid(true), _path(std::move(path)), _fd(get_fd(_path)),
         _dev_info(get_dev_info(_fd)), _name(get_name(_fd)),
-        _report_desc(getReportDescriptor(_fd)), _io_monitor(monitor->ioMonitor()) {
+        _report_desc(getReportDescriptor(_fd)), _io_monitor(monitor->ioMonitor()),
+        _event_handlers(std::make_shared<EventHandlerList<RawDevice>>()) {
     _io_monitor->add(_fd, {
             [this]() { _readReports(); },
             [this]() { _valid = false; },
@@ -154,15 +155,10 @@ void RawDevice::sendReport(const std::vector<uint8_t>& report) {
                                 "sendReport write failed");
 }
 
-RawDevice::EvHandlerId RawDevice::addEventHandler(RawEventHandler handler) {
+EventHandlerLock<RawDevice> RawDevice::addEventHandler(RawEventHandler handler) {
-    std::unique_lock<std::shared_mutex> lock(_event_handler_mutex);
+    std::unique_lock<std::shared_mutex> lock(_event_handlers->mutex);
-    _event_handlers.emplace_front(std::move(handler));
+    _event_handlers->list.emplace_front(std::move(handler));
-    return _event_handlers.cbegin();
+    return {_event_handlers, _event_handlers->list.cbegin()};
-}
-
-void RawDevice::removeEventHandler(RawDevice::EvHandlerId id) {
-    std::unique_lock<std::shared_mutex> lock(_event_handler_mutex);
-    _event_handlers.erase(id);
 }
 
 void RawDevice::_readReports() {
@@ -185,8 +181,8 @@ void RawDevice::_readReports() {
 }
 
 void RawDevice::_handleEvent(const std::vector<uint8_t>& report) {
-    std::shared_lock<std::shared_mutex> lock(_event_handler_mutex);
+    std::shared_lock<std::shared_mutex> lock(_event_handlers->mutex);
-    for (auto& handler: _event_handlers)
+    for (auto& handler : _event_handlers->list)
         if (handler.condition(report))
             handler.callback(report);
 }

src/logid/backend/raw/RawDevice.h

@@ -19,6 +19,8 @@
 #ifndef LOGID_BACKEND_RAWDEVICE_H
 #define LOGID_BACKEND_RAWDEVICE_H
 
+#include <backend/raw/EventHandler.h>
+#include <backend/EventHandlerList.h>
 #include <string>
 #include <vector>
 #include <shared_mutex>
@@ -26,7 +28,6 @@
 #include <future>
 #include <set>
 #include <list>
-#include <backend/raw/EventHandler.h>
 
 namespace logid::backend::raw {
     class DeviceMonitor;
@@ -36,15 +37,14 @@ namespace logid::backend::raw {
     class RawDevice {
     public:
         static constexpr int max_data_length = 32;
-        typedef std::list<RawEventHandler>::const_iterator EvHandlerId;
+        typedef RawEventHandler EventHandler;
 
         struct dev_info {
             int16_t vid;
             int16_t pid;
         };
 
-        RawDevice(std::string path,
+        RawDevice(std::string path, const std::shared_ptr<DeviceMonitor>& monitor);
-                  const std::shared_ptr<DeviceMonitor>& monitor);
 
         ~RawDevice() noexcept;
 
@@ -65,9 +65,7 @@ namespace logid::backend::raw {
 
         void sendReport(const std::vector<uint8_t>& report);
 
-        EvHandlerId addEventHandler(RawEventHandler handler);
+        [[nodiscard]] EventHandlerLock<RawDevice> addEventHandler(RawEventHandler handler);
-
-        void removeEventHandler(EvHandlerId id);
 
     private:
         void _readReports();
@@ -82,8 +80,7 @@ namespace logid::backend::raw {
 
         std::shared_ptr<IOMonitor> _io_monitor;
 
-        std::list<RawEventHandler> _event_handlers;
+        std::shared_ptr<EventHandlerList<RawDevice>> _event_handlers;
-        std::shared_mutex _event_handler_mutex;
 
         void _handleEvent(const std::vector<uint8_t>& report);
     };

src/logid/features/DeviceStatus.cpp

@@ -38,17 +38,12 @@ DeviceStatus::DeviceStatus(logid::Device* dev) : DeviceFeature(dev) {
     }
 }
 
-DeviceStatus::~DeviceStatus() noexcept {
-    if (_ev_handler.has_value())
-        _device->hidpp20().removeEventHandler(_ev_handler.value());
-}
-
 void DeviceStatus::configure() {
     // Do nothing
 }
 
 void DeviceStatus::listen() {
-    if (!_ev_handler.has_value()) {
+    if (_ev_handler.empty()) {
         _ev_handler = _device->hidpp20().addEventHandler(
                 {
                         [index = _wireless_device_status->featureIndex()](

src/logid/features/DeviceStatus.h

@@ -28,16 +28,13 @@ namespace logid::features {
     public:
         explicit DeviceStatus(Device* dev);
 
-        ~DeviceStatus() noexcept override;
-
         void configure() final;
 
         void listen() final;
 
     private:
-        std::optional<backend::hidpp::Device::EvHandlerId> _ev_handler;
+        EventHandlerLock<backend::hidpp::Device> _ev_handler;
-        std::shared_ptr<backend::hidpp20::WirelessDeviceStatus>
+        std::shared_ptr<backend::hidpp20::WirelessDeviceStatus> _wireless_device_status;
-                _wireless_device_status;
     };
 }
 

src/logid/features/HiresScroll.cpp

@@ -74,11 +74,6 @@ HiresScroll::HiresScroll(Device* dev) :
     _ipc_interface = dev->ipcNode()->make_interface<IPC>(this);
 }
 
-HiresScroll::~HiresScroll() noexcept {
-    if (_ev_handler.has_value())
-        _device->hidpp20().removeEventHandler(_ev_handler.value());
-}
-
 void HiresScroll::configure() {
     std::shared_lock lock(_config_mutex);
     _configure();
@@ -93,7 +88,7 @@ void HiresScroll::_configure() {
 
 void HiresScroll::listen() {
     std::shared_lock lock(_config_mutex);
-    if (!_ev_handler.has_value()) {
+    if (_ev_handler.empty()) {
         _ev_handler = _device->hidpp20().addEventHandler(
                 {
                         [index = _hires_scroll->featureIndex()](

src/logid/features/HiresScroll.h

@@ -32,8 +32,6 @@ namespace logid::features {
     public:
         explicit HiresScroll(Device* dev);
 
-        ~HiresScroll() noexcept override;
-
         void configure() final;
 
         void listen() final;
@@ -43,7 +41,7 @@ namespace logid::features {
         void setMode(uint8_t mode);
 
     private:
-        std::optional<backend::hidpp::Device::EvHandlerId> _ev_handler;
+        EventHandlerLock<backend::hidpp::Device> _ev_handler;
 
         void _makeGesture(std::shared_ptr<actions::Gesture>& gesture,
                           std::optional<config::Gesture>& config,

src/logid/features/RemapButton.cpp

@@ -91,18 +91,13 @@ RemapButton::RemapButton(Device* dev) : DeviceFeature(dev),
     }
 }
 
-RemapButton::~RemapButton() noexcept {
-    if (_ev_handler.has_value())
-        _device->hidpp20().removeEventHandler(_ev_handler.value());
-}
-
 void RemapButton::configure() {
     for (const auto& button: _buttons)
         button.second->configure();
 }
 
 void RemapButton::listen() {
-    if (!_ev_handler.has_value()) {
+    if (_ev_handler.empty()) {
         _ev_handler = _device->hidpp20().addEventHandler(
                 {
                         [index = _reprog_controls->featureIndex()](

src/logid/features/RemapButton.h

@@ -85,8 +85,6 @@ namespace logid::features {
     public:
         explicit RemapButton(Device* dev);
 
-        ~RemapButton() noexcept override;
-
         void configure() final;
 
         void listen() final;
@@ -114,7 +112,7 @@ namespace logid::features {
         };
 
         std::shared_ptr<IPC> _ipc_interface;
-        std::optional<backend::hidpp::Device::EvHandlerId> _ev_handler;
+        EventHandlerLock<backend::hidpp::Device> _ev_handler;
     };
 }
 

src/logid/features/ThumbWheel.cpp

@@ -109,11 +109,6 @@ ThumbWheel::ThumbWheel(Device* dev) : DeviceFeature(dev), _wheel_info(),
     _ipc_interface = dev->ipcNode()->make_interface<IPC>(this);
 }
 
-ThumbWheel::~ThumbWheel() noexcept {
-    if (_ev_handler.has_value())
-        _device->hidpp20().removeEventHandler(_ev_handler.value());
-}
-
 void ThumbWheel::configure() {
     std::shared_lock lock(_config_mutex);
     if (_config.has_value()) {
@@ -124,7 +119,7 @@ void ThumbWheel::configure() {
 }
 
 void ThumbWheel::listen() {
-    if (!_ev_handler.has_value()) {
+    if (_ev_handler.empty()) {
         _ev_handler = _device->hidpp20().addEventHandler(
                 {
                         [index = _thumb_wheel->featureIndex()]

src/logid/features/ThumbWheel.h

@@ -28,8 +28,6 @@ namespace logid::features {
     public:
         explicit ThumbWheel(Device* dev);
 
-        ~ThumbWheel() noexcept override;
-
         void configure() final;
 
         void listen() final;
@@ -88,7 +86,7 @@ namespace logid::features {
         mutable std::shared_mutex _config_mutex;
         std::optional<config::ThumbWheel>& _config;
 
-        std::optional<backend::hidpp::Device::EvHandlerId> _ev_handler;
+        EventHandlerLock<backend::hidpp::Device> _ev_handler;
     };
 }