diff --git a/src/bluez/adapter1.h b/src/bluez/adapter1.h index 5aaa761..b96f1e5 100644 --- a/src/bluez/adapter1.h +++ b/src/bluez/adapter1.h @@ -60,65 +60,43 @@ class Adapter1 final const sdbus::InterfaceName& interfaceName, const std::map& changedProperties, const std::vector& invalidatedProperties) override { - if (const auto key = sdbus::MemberName("Address"); - changedProperties.contains(key)) { - properties_.address = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("AddressType"); - changedProperties.contains(key)) { - properties_.address_type = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Alias"); - changedProperties.contains(key)) { - properties_.alias = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Class"); - changedProperties.contains(key)) { - properties_.class_type = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Discoverable"); - changedProperties.contains(key)) { - properties_.discoverable = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("DiscoverableTimeout"); - changedProperties.contains(key)) { - properties_.discoverable_timeout = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Discovering"); - changedProperties.contains(key)) { - properties_.discovering = changedProperties.at(key).get(); + // Iterate the changed properties once and dispatch on the key. A + // PropertiesChanged signal usually carries only a handful of properties, + // so this avoids scanning every known property (and the per-property map + // lookup + MemberName allocation) on every signal. + for (const auto& [key, value] : changedProperties) { + if (key == "Address") { + properties_.address = value.get(); + } else if (key == "AddressType") { + properties_.address_type = value.get(); + } else if (key == "Alias") { + properties_.alias = value.get(); + } else if (key == "Class") { + properties_.class_type = value.get(); + } else if (key == "Discoverable") { + properties_.discoverable = value.get(); + } else if (key == "DiscoverableTimeout") { + properties_.discoverable_timeout = value.get(); + } else if (key == "Discovering") { + properties_.discovering = value.get(); - if (!properties_.discovering) { - this->StartDiscovery(); + if (!properties_.discovering) { + this->StartDiscovery(); + } + } else if (key == "Modalias") { + properties_.modalias = value.get(); + } else if (key == "Name") { + properties_.name = value.get(); + } else if (key == "Pairable") { + properties_.pairable = value.get(); + } else if (key == "PairableTimeout") { + properties_.pairable_timeout = value.get(); + } else if (key == "Powered") { + properties_.powered = value.get(); + } else if (key == "UUIDs") { + properties_.uuids = value.get>(); } } - if (const auto key = sdbus::MemberName("Modalias"); - changedProperties.contains(key)) { - properties_.modalias = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Name"); - changedProperties.contains(key)) { - properties_.name = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Pairable"); - changedProperties.contains(key)) { - properties_.pairable = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("PairableTimeout"); - changedProperties.contains(key)) { - properties_.pairable_timeout = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Powered"); - changedProperties.contains(key)) { - properties_.powered = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UUIDs"); - changedProperties.contains(key)) { - properties_.uuids = - changedProperties.at(key).get>(); - } } }; diff --git a/src/bluez/device1.h b/src/bluez/device1.h index 372cb7d..62e5782 100644 --- a/src/bluez/device1.h +++ b/src/bluez/device1.h @@ -85,70 +85,44 @@ class Device1 final : public sdbus::ProxyInterfaces& changedProperties, const std::vector& invalidatedProperties) override { - if (const auto key = sdbus::MemberName("Adapter"); - changedProperties.contains(key)) { - properties_.adapter = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Address"); - changedProperties.contains(key)) { - properties_.address = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("AddressType"); - changedProperties.contains(key)) { - properties_.address_type = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Bonded"); - changedProperties.contains(key)) { - properties_.bonded = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Blocked"); - changedProperties.contains(key)) { - properties_.blocked = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Connected"); - changedProperties.contains(key)) { - properties_.connected = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("LegacyPairing"); - changedProperties.contains(key)) { - properties_.legacy_pairing = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Paired"); - changedProperties.contains(key)) { - properties_.paired = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Modalias"); - changedProperties.contains(key)) { - properties_.modalias = - parse_modalias(changedProperties.at(key).get()); - } - if (const auto key = sdbus::MemberName("Name"); - changedProperties.contains(key)) { - properties_.name = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ServiceData"); - changedProperties.contains(key)) { - properties_.service_data = - changedProperties.at(key) - .get>(); - } - if (const auto key = sdbus::MemberName("RSSI"); - changedProperties.contains(key)) { - properties_.rssi = changedProperties.at(key).get(); - LOG_DEBUG("RSSI: {}", properties_.rssi); - } - if (const auto key = sdbus::MemberName("ServicesResolved"); - changedProperties.contains(key)) { - properties_.services_resolved = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Trusted"); - changedProperties.contains(key)) { - properties_.trusted = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UUIDs"); - changedProperties.contains(key)) { - properties_.uuids = - changedProperties.at(key).get>(); + // Iterate the changed properties once and dispatch on the key. A + // PropertiesChanged signal usually carries only a handful of properties, + // so this avoids scanning every known property (and the per-property map + // lookup + MemberName allocation) on every signal. + for (const auto& [key, value] : changedProperties) { + if (key == "Adapter") { + properties_.adapter = value.get(); + } else if (key == "Address") { + properties_.address = value.get(); + } else if (key == "AddressType") { + properties_.address_type = value.get(); + } else if (key == "Bonded") { + properties_.bonded = value.get(); + } else if (key == "Blocked") { + properties_.blocked = value.get(); + } else if (key == "Connected") { + properties_.connected = value.get(); + } else if (key == "LegacyPairing") { + properties_.legacy_pairing = value.get(); + } else if (key == "Paired") { + properties_.paired = value.get(); + } else if (key == "Modalias") { + properties_.modalias = parse_modalias(value.get()); + } else if (key == "Name") { + properties_.name = value.get(); + } else if (key == "ServiceData") { + properties_.service_data = + value.get>(); + } else if (key == "RSSI") { + properties_.rssi = value.get(); + LOG_DEBUG("RSSI: {}", properties_.rssi); + } else if (key == "ServicesResolved") { + properties_.services_resolved = value.get(); + } else if (key == "Trusted") { + properties_.trusted = value.get(); + } else if (key == "UUIDs") { + properties_.uuids = value.get>(); + } } #if 0 Utils::print_changed_properties(interfaceName, changedProperties, diff --git a/src/bluez/gatt_characteristic1.h b/src/bluez/gatt_characteristic1.h index 5ab4d6c..346594b 100644 --- a/src/bluez/gatt_characteristic1.h +++ b/src/bluez/gatt_characteristic1.h @@ -35,29 +35,30 @@ class GattCharacteristic1 final const sdbus::ObjectPath(&objectPath), const std::map& properties) : ProxyInterfaces{connection, destination, objectPath} { - if (const auto key = sdbus::MemberName("Flags"); properties.contains(key)) { - properties_.flags = properties.at(key).get>(); - } - if (isNotifyFlagSet()) { - if (const auto key = sdbus::MemberName("NotifyAcquired"); - properties.contains(key)) { - properties_.notify_acquired = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Notifying"); - properties.contains(key)) { - properties_.notifying = properties.at(key).get(); + // Iterate the provided properties once and dispatch on the key. This avoids + // scanning every known property (and the per-property map lookup + + // MemberName allocation). "Flags" sorts before "NotifyAcquired"/"Notifying" + // in the map, so it is assigned before those guarded assignments are + // reached, preserving the original isNotifyFlagSet() behavior. + for (const auto& [key, value] : properties) { + if (key == "Flags") { + properties_.flags = value.get>(); + } else if (key == "NotifyAcquired") { + if (isNotifyFlagSet()) { + properties_.notify_acquired = value.get(); + } + } else if (key == "Notifying") { + if (isNotifyFlagSet()) { + properties_.notifying = value.get(); + } + } else if (key == "Service") { + properties_.service = value.get(); + } else if (key == "UUID") { + properties_.uuid = value.get(); + } else if (key == "Value") { + properties_.value = value.get>(); } } - if (const auto key = sdbus::MemberName("Service"); - properties.contains(key)) { - properties_.service = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UUID"); properties.contains(key)) { - properties_.uuid = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Value"); properties.contains(key)) { - properties_.value = properties.at(key).get>(); - } } [[nodiscard]] bool isNotifyFlagSet() const { diff --git a/src/bluez/gatt_descriptor1.h b/src/bluez/gatt_descriptor1.h index c542939..c12a8ec 100644 --- a/src/bluez/gatt_descriptor1.h +++ b/src/bluez/gatt_descriptor1.h @@ -31,15 +31,17 @@ class GattDescriptor1 final const sdbus::ObjectPath(&objectPath), const std::map& properties) : ProxyInterfaces{connection, destination, objectPath} { - if (const auto key = sdbus::MemberName("Characteristic"); - properties.contains(key)) { - properties_.characteristic = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UUID"); properties.contains(key)) { - properties_.uuid = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Value"); properties.contains(key)) { - properties_.value = properties.at(key).get>(); + // Iterate the provided properties once and dispatch on the key. This avoids + // scanning every known property (and the per-property map lookup + + // MemberName allocation). + for (const auto& [key, value] : properties) { + if (key == "Characteristic") { + properties_.characteristic = value.get(); + } else if (key == "UUID") { + properties_.uuid = value.get(); + } else if (key == "Value") { + properties_.value = value.get>(); + } } } diff --git a/src/bluez/gatt_service1.h b/src/bluez/gatt_service1.h index b2aea33..d719c3a 100644 --- a/src/bluez/gatt_service1.h +++ b/src/bluez/gatt_service1.h @@ -32,21 +32,19 @@ class GattService1 const sdbus::ObjectPath(&objectPath), const std::map& properties) : ProxyInterfaces{connection, destination, objectPath} { - if (const auto key = sdbus::MemberName("Device"); - properties.contains(key)) { - properties_.device = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Includes"); - properties.contains(key)) { - properties_.includes = - properties.at(key).get>(); - } - if (const auto key = sdbus::MemberName("Primary"); - properties.contains(key)) { - properties_.primary = properties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UUID"); properties.contains(key)) { - properties_.uuid = properties.at(key).get(); + // Iterate the provided properties once and dispatch on the key. This avoids + // scanning every known property (and the per-property map lookup + + // MemberName allocation). + for (const auto& [key, value] : properties) { + if (key == "Device") { + properties_.device = value.get(); + } else if (key == "Includes") { + properties_.includes = value.get>(); + } else if (key == "Primary") { + properties_.primary = value.get(); + } else if (key == "UUID") { + properties_.uuid = value.get(); + } } } diff --git a/src/bluez/horipad_steam/horipad_steam.cc b/src/bluez/horipad_steam/horipad_steam.cc index 72bd784..17b2bd8 100644 --- a/src/bluez/horipad_steam/horipad_steam.cc +++ b/src/bluez/horipad_steam/horipad_steam.cc @@ -45,6 +45,7 @@ HoripadSteam::HoripadSteam(sdbus::IConnection& connection) sub_system ? sub_system : ""); if (std::strcmp(sub_system, "hidraw") == 0) { if (std::strcmp(action, "remove") == 0) { + std::scoped_lock reader_lock(input_reader_mutex_); if (input_reader_) { input_reader_->stop(); input_reader_.reset(); @@ -155,6 +156,7 @@ void HoripadSteam::onInterfacesAdded( if (const std::string hidraw_device = FindHidDevice(hidraw_device_key); !hidraw_device.empty()) { LOG_INFO("Adding hidraw device: {}", hidraw_device_key); + std::scoped_lock reader_lock(input_reader_mutex_); if (!input_reader_) { input_reader_ = std::make_unique(hidraw_device); input_reader_->start(); diff --git a/src/bluez/horipad_steam/horipad_steam.h b/src/bluez/horipad_steam/horipad_steam.h index cb1854d..7ad88d0 100644 --- a/src/bluez/horipad_steam/horipad_steam.h +++ b/src/bluez/horipad_steam/horipad_steam.h @@ -57,6 +57,10 @@ class HoripadSteam final std::mutex input1_mutex_; std::map> input1_; + // Guards input_reader_, which is created/started from onInterfacesAdded + // (D-Bus event-loop or main thread) and stopped/reset from the udev monitor + // worker thread. + std::mutex input_reader_mutex_; std::unique_ptr input_reader_; void onInterfacesAdded( diff --git a/src/bluez/horipad_steam/input_reader.cc b/src/bluez/horipad_steam/input_reader.cc index fcf34e9..8420bf2 100644 --- a/src/bluez/horipad_steam/input_reader.cc +++ b/src/bluez/horipad_steam/input_reader.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -19,6 +20,7 @@ #include #include +#include #include #include "../../utils/logging.h" @@ -26,25 +28,43 @@ #include "input_reader.h" InputReader::InputReader(std::string device) - : device_(std::move(device)), stop_flag_(false) {} + : device_(std::move(device)), + stop_flag_(false), + stop_event_fd_(::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { + if (!stop_event_fd_.valid()) { + LOG_ERROR("Failed to create eventfd: {}", strerror(errno)); + } +} void InputReader::start() { LOG_DEBUG("InputReader start: {}", device_); + if (thread_.joinable()) { + return; // already running + } stop_flag_ = false; - read_input(); + thread_ = std::thread([this] { read_input(); }); } void InputReader::stop() { LOG_DEBUG("InputReader stop: {}", device_); stop_flag_ = true; + // Wake the blocking epoll_wait so the loop observes stop_flag_ immediately. + if (stop_event_fd_.valid()) { + constexpr std::uint64_t one = 1; + if (::write(stop_event_fd_.get(), &one, sizeof(one)) < 0) { + LOG_ERROR("Failed to signal stop eventfd: {}", strerror(errno)); + } + } } InputReader::~InputReader() { stop(); + if (thread_.joinable()) { + thread_.join(); + } } -// NOLINTNEXTLINE(readability-static-accessed-through-instance) -InputReader::Task InputReader::read_input() { +void InputReader::read_input() { LOG_DEBUG("hidraw device: {}", device_); const UniqueFd fd(open(device_.c_str(), O_RDWR)); @@ -114,13 +134,70 @@ InputReader::Task InputReader::read_input() { os << CustomHexdump<400, false>(std::data(rpt_desc.value), rpt_desc.size); LOG_INFO(os.str()); + // Wait on both the hidraw fd and the stop eventfd so a blocking read can + // be interrupted immediately when stop() is called from another thread. + const UniqueFd epoll_fd(epoll_create1(EPOLL_CLOEXEC)); + if (!epoll_fd.valid()) { + LOG_ERROR("epoll_create1 failed: {}", strerror(errno)); + break; + } + epoll_event ev{}; + ev.events = EPOLLIN; + ev.data.fd = fd.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, fd.get(), &ev) == -1) { + LOG_ERROR("epoll_ctl(hidraw) failed: {}", strerror(errno)); + break; + } + if (stop_event_fd_.valid()) { + ev.data.fd = stop_event_fd_.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, stop_event_fd_.get(), &ev) == + -1) { + LOG_ERROR("epoll_ctl(stop) failed: {}", strerror(errno)); + break; + } + } + while (!stop_flag_) { + std::array poll_events{}; + const int nfds = epoll_wait(epoll_fd.get(), poll_events.data(), + poll_events.size(), -1); + if (nfds == -1) { + if (errno == EINTR) { + continue; + } + LOG_ERROR("epoll_wait failed: {}", strerror(errno)); + break; + } + + bool stop_requested = false; + bool data_ready = false; + for (int i = 0; i < nfds; ++i) { + if (stop_event_fd_.valid() && + poll_events.at(i).data.fd == stop_event_fd_.get()) { + stop_requested = true; + } else if (poll_events.at(i).data.fd == fd.get()) { + data_ready = true; + } + } + if (stop_requested) { + break; + } + if (!data_ready) { + continue; + } + std::array buffer{}; - ssize_t result = 0; - if (result = read(fd.get(), buffer.data(), buffer.size()); result < 0) { + const ssize_t result = read(fd.get(), buffer.data(), buffer.size()); + if (result < 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } LOG_ERROR("read failed: {}", strerror(errno)); break; } + if (result == 0) { + continue; + } if (raw_dev_info.product == 0x01ab || raw_dev_info.product == 0x0196) { if (const auto report_id = buffer.at(0); report_id == 7) { @@ -151,10 +228,7 @@ InputReader::Task InputReader::read_input() { break; } - // fd is automatically closed by UniqueFd destructor - stop(); - - co_return; // NOLINT(readability-static-accessed-through-instance) + // fd is automatically closed by UniqueFd destructor. } std::string InputReader::dpad_to_string(const Direction dpad) { @@ -213,20 +287,20 @@ void InputReader::PrintInputReport7(const inputReport07_t& input_report07) { void InputReader::PrintInputReport10(const inputReport10_t& input_report10) { std::ostringstream os; os << CustomHexdump<400, false>(std::data(input_report10.VEN_Gamepad0024), - sizeof(inputReport10_t)); + sizeof(input_report10.VEN_Gamepad0024)); LOG_INFO("Input Report 10: {}", os.str()); } void InputReader::PrintInputReport12(const inputReport12_t& input_report12) { std::ostringstream os; os << CustomHexdump<400, false>(std::data(input_report12.VEN_Gamepad0022), - sizeof(inputReport10_t)); - LOG_INFO("Input Report 10: {}", os.str()); + sizeof(input_report12.VEN_Gamepad0022)); + LOG_INFO("Input Report 12: {}", os.str()); } void InputReader::PrintInputReport14(const inputReport14_t& input_report14) { std::ostringstream os; os << CustomHexdump<400, false>(std::data(input_report14.VEN_Gamepad0026), - sizeof(inputReport10_t)); + sizeof(input_report14.VEN_Gamepad0026)); LOG_INFO("Input Report 14: {}", os.str()); } diff --git a/src/bluez/horipad_steam/input_reader.h b/src/bluez/horipad_steam/input_reader.h index 9ce18c2..800fa14 100644 --- a/src/bluez/horipad_steam/input_reader.h +++ b/src/bluez/horipad_steam/input_reader.h @@ -16,8 +16,9 @@ #define SRC_BLUEZ_XBOX_CONTROLLER_INPUT_READER_HPP_ #include -#include +#include +#include "../hidraw.hpp" #include "horipad_stream_01ab_0196.h" class InputReader { @@ -43,20 +44,16 @@ class InputReader { ~InputReader(); private: - struct Task { - struct promise_type { - static Task get_return_object() { return {}; } - static std::suspend_never initial_suspend() { return {}; } - static std::suspend_never final_suspend() noexcept { return {}; } - static void return_void() {} - static void unhandled_exception() { std::terminate(); } - }; - }; - std::string device_; std::atomic stop_flag_; - - Task read_input(); + // eventfd used to interrupt the blocking read loop immediately on stop(). + UniqueFd stop_event_fd_; + // Worker thread that owns the blocking read loop. Joined in the destructor + // before any other member is torn down, so the loop can never outlive this + // object (no use-after-free) and never blocks the D-Bus/main thread. + std::thread thread_; + + void read_input(); static std::string dpad_to_string(Direction dpad); diff --git a/src/bluez/le_advertising_manager1.h b/src/bluez/le_advertising_manager1.h index c165012..ef47fc3 100644 --- a/src/bluez/le_advertising_manager1.h +++ b/src/bluez/le_advertising_manager1.h @@ -49,25 +49,21 @@ class LEAdvertisingManager1 final const sdbus::InterfaceName& interfaceName, const std::map& changedProperties, const std::vector& invalidatedProperties) override { - if (const auto key = sdbus::MemberName("ActiveInstances"); - changedProperties.contains(key)) { - properties_.active_instances = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("SupportedIncludes"); - changedProperties.contains(key)) { - properties_.supported_includes = - changedProperties.at(key).get>(); - } - if (const auto key = sdbus::MemberName("SupportedInstances"); - changedProperties.contains(key)) { - properties_.supported_instances = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("SupportedSecondaryChannels"); - changedProperties.contains(key)) { - properties_.supported_secondary_channels = - changedProperties.at(key).get>(); + // Iterate the changed properties once and dispatch on the key. A + // PropertiesChanged signal usually carries only a handful of properties, + // so this avoids scanning every known property (and the per-property map + // lookup + MemberName allocation) on every signal. + for (const auto& [key, value] : changedProperties) { + if (key == "ActiveInstances") { + properties_.active_instances = value.get(); + } else if (key == "SupportedIncludes") { + properties_.supported_includes = value.get>(); + } else if (key == "SupportedInstances") { + properties_.supported_instances = value.get(); + } else if (key == "SupportedSecondaryChannels") { + properties_.supported_secondary_channels = + value.get>(); + } } } }; diff --git a/src/bluez/ps5_dual_sense/dual_sense.cc b/src/bluez/ps5_dual_sense/dual_sense.cc index f939533..eaa7ad1 100644 --- a/src/bluez/ps5_dual_sense/dual_sense.cc +++ b/src/bluez/ps5_dual_sense/dual_sense.cc @@ -44,6 +44,7 @@ DualSense::DualSense(sdbus::IConnection& connection) sub_system ? sub_system : ""); if (std::strcmp(sub_system, "hidraw") == 0) { if (std::strcmp(action, "remove") == 0) { + std::scoped_lock reader_lock(input_reader_mutex_); if (input_reader_) { input_reader_->stop(); input_reader_.reset(); @@ -157,6 +158,7 @@ void DualSense::onInterfacesAdded( if (const std::string hidraw_device = FindHidDevice(hidraw_device_key); !hidraw_device.empty()) { LOG_INFO("Adding hidraw device: {}", hidraw_device_key); + std::scoped_lock reader_lock(input_reader_mutex_); if (!input_reader_) { input_reader_ = std::make_unique(hidraw_device); input_reader_->start(); diff --git a/src/bluez/ps5_dual_sense/dual_sense.h b/src/bluez/ps5_dual_sense/dual_sense.h index b06d75f..3034273 100644 --- a/src/bluez/ps5_dual_sense/dual_sense.h +++ b/src/bluez/ps5_dual_sense/dual_sense.h @@ -62,6 +62,10 @@ class DualSense final std::mutex upower_display_devices_mutex_; std::map> upower_clients_; + // Guards input_reader_, which is created/started from onInterfacesAdded + // (D-Bus event-loop or main thread) and stopped/reset from the udev monitor + // worker thread. + std::mutex input_reader_mutex_; std::unique_ptr input_reader_; void onInterfacesAdded( diff --git a/src/bluez/ps5_dual_sense/input_reader.cc b/src/bluez/ps5_dual_sense/input_reader.cc index 393d8d8..d4b8a17 100644 --- a/src/bluez/ps5_dual_sense/input_reader.cc +++ b/src/bluez/ps5_dual_sense/input_reader.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -20,6 +21,7 @@ #include #include +#include #include #include "../../utils/logging.h" @@ -27,25 +29,43 @@ #include "input_reader.h" InputReader::InputReader(std::string device) - : device_(std::move(device)), stop_flag_(false) {} + : device_(std::move(device)), + stop_flag_(false), + stop_event_fd_(::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { + if (!stop_event_fd_.valid()) { + LOG_ERROR("Failed to create eventfd: {}", strerror(errno)); + } +} void InputReader::start() { LOG_DEBUG("InputReader start: {}", device_); + if (thread_.joinable()) { + return; // already running + } stop_flag_ = false; - read_input(); + thread_ = std::thread([this] { read_input(); }); } void InputReader::stop() { LOG_DEBUG("InputReader stop: {}", device_); stop_flag_ = true; + // Wake the blocking epoll_wait so the loop observes stop_flag_ immediately. + if (stop_event_fd_.valid()) { + constexpr std::uint64_t one = 1; + if (::write(stop_event_fd_.get(), &one, sizeof(one)) < 0) { + LOG_ERROR("Failed to signal stop eventfd: {}", strerror(errno)); + } + } } InputReader::~InputReader() { stop(); + if (thread_.joinable()) { + thread_.join(); + } } -// NOLINTNEXTLINE(readability-static-accessed-through-instance) -InputReader::Task InputReader::read_input() { +void InputReader::read_input() { LOG_DEBUG("hidraw device: {}", device_); const UniqueFd fd(open(device_.c_str(), O_RDWR)); @@ -121,24 +141,86 @@ InputReader::Task InputReader::read_input() { GetControllerMacAll(fd.get(), controller_and_host_mac_); GetControllerVersion(fd.get(), version_); + // Wait on both the hidraw fd and the stop eventfd so a blocking read can + // be interrupted immediately when stop() is called from another thread. + const UniqueFd epoll_fd(epoll_create1(EPOLL_CLOEXEC)); + if (!epoll_fd.valid()) { + LOG_ERROR("epoll_create1 failed: {}", strerror(errno)); + break; + } + epoll_event ev{}; + ev.events = EPOLLIN; + ev.data.fd = fd.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, fd.get(), &ev) == -1) { + LOG_ERROR("epoll_ctl(hidraw) failed: {}", strerror(errno)); + break; + } + if (stop_event_fd_.valid()) { + ev.data.fd = stop_event_fd_.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, stop_event_fd_.get(), &ev) == + -1) { + LOG_ERROR("epoll_ctl(stop) failed: {}", strerror(errno)); + break; + } + } + while (!stop_flag_) { - std::array buffer{}; - ssize_t result = 0; - if (result = read(fd.get(), buffer.data(), buffer.size()); result < 0) { - LOG_ERROR("GetInputReport4 failed: {}", strerror(errno)); + std::array poll_events{}; + const int nfds = epoll_wait(epoll_fd.get(), poll_events.data(), + poll_events.size(), -1); + if (nfds == -1) { + if (errno == EINTR) { + continue; + } + LOG_ERROR("epoll_wait failed: {}", strerror(errno)); + break; + } + + bool stop_requested = false; + bool data_ready = false; + for (int i = 0; i < nfds; ++i) { + if (stop_event_fd_.valid() && + poll_events.at(i).data.fd == stop_event_fd_.get()) { + stop_requested = true; + } else if (poll_events.at(i).data.fd == fd.get()) { + data_ready = true; + } + } + if (stop_requested) { + break; + } + if (!data_ready) { + continue; + } + + // Buffer sized to the largest report (ReportIn31) so no report is + // truncated on read. + std::array + buffer{}; + const ssize_t result = read(fd.get(), buffer.data(), buffer.size()); + if (result < 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } + LOG_ERROR("read failed: {}", strerror(errno)); break; } + if (result == 0) { + continue; + } + const auto bytes_read = static_cast(result); if (raw_dev_info.product == 0x0CE6) { if (const auto report_id = buffer.at(0); report_id == 1) { USBGetStateData input_report01{}; std::memcpy(&input_report01, buffer.data(), - std::min(sizeof(USBGetStateData), buffer.size())); + std::min(sizeof(USBGetStateData), bytes_read)); PrintControllerStateUsb(input_report01, hw_cal_data_); } else if (report_id == 49) { ReportIn31 input_report31{}; std::memcpy(&input_report31, buffer.data(), - std::min(sizeof(ReportIn31), buffer.size())); + std::min(sizeof(ReportIn31), bytes_read)); if (input_report31.Data.HasHID) { LOG_INFO("[ReportIn31] Has HID"); PrintControllerStateUsb(input_report31.Data.State.StateData, @@ -154,10 +236,7 @@ InputReader::Task InputReader::read_input() { break; } - // fd is automatically closed by UniqueFd destructor - stop(); - - co_return; // NOLINT(readability-static-accessed-through-instance) + // fd is automatically closed by UniqueFd destructor. } int InputReader::GetControllerMacAll(const int fd, diff --git a/src/bluez/ps5_dual_sense/input_reader.h b/src/bluez/ps5_dual_sense/input_reader.h index 65a52ab..55b7f94 100644 --- a/src/bluez/ps5_dual_sense/input_reader.h +++ b/src/bluez/ps5_dual_sense/input_reader.h @@ -17,7 +17,7 @@ #include #include -#include +#include #include "../hidraw.hpp" #include "dual_sense_0ce6.h" @@ -33,16 +33,6 @@ class InputReader { ~InputReader(); private: - struct Task { - struct promise_type { - static Task get_return_object() { return {}; } - static std::suspend_never initial_suspend() { return {}; } - static std::suspend_never final_suspend() noexcept { return {}; } - static void return_void() {} - static void unhandled_exception() { std::terminate(); } - }; - }; - struct CalibrationData { std::int32_t abs_code; std::int16_t bias; @@ -57,12 +47,19 @@ class InputReader { std::string device_; std::atomic stop_flag_; + // eventfd used to interrupt the blocking read loop immediately on stop(). + UniqueFd stop_event_fd_; ReportFeatureInMacAll controller_and_host_mac_{}; ReportFeatureInVersion version_{}; HardwareCalibrationData hw_cal_data_{}; - Task read_input(); + // Worker thread that owns the blocking read loop. Joined in the destructor + // before any other member is torn down, so the loop can never outlive this + // object (no use-after-free) and never blocks the D-Bus/main thread. + std::thread thread_; + + void read_input(); static std::string dpad_to_string(Direction dpad); static std::string power_state_to_string(PowerState state); diff --git a/src/bluez/xbox_controller/input_reader.cc b/src/bluez/xbox_controller/input_reader.cc index ddc30b0..4b6c19c 100644 --- a/src/bluez/xbox_controller/input_reader.cc +++ b/src/bluez/xbox_controller/input_reader.cc @@ -12,14 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include -#include #include #include #include #include +#include #include #include "../../utils/logging.h" @@ -27,25 +28,43 @@ #include "input_reader.h" InputReader::InputReader(std::string device) - : device_(std::move(device)), stop_flag_(false) {} + : device_(std::move(device)), + stop_flag_(false), + stop_event_fd_(::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { + if (!stop_event_fd_.valid()) { + LOG_ERROR("Failed to create eventfd: {}", strerror(errno)); + } +} void InputReader::start() { LOG_DEBUG("InputReader start: {}", device_); + if (thread_.joinable()) { + return; // already running + } stop_flag_ = false; - read_input(); + thread_ = std::thread([this] { read_input(); }); } void InputReader::stop() { LOG_DEBUG("InputReader stop: {}", device_); stop_flag_ = true; + // Wake the blocking epoll_wait so the loop observes stop_flag_ immediately. + if (stop_event_fd_.valid()) { + constexpr std::uint64_t one = 1; + if (::write(stop_event_fd_.get(), &one, sizeof(one)) < 0) { + LOG_ERROR("Failed to signal stop eventfd: {}", strerror(errno)); + } + } } InputReader::~InputReader() { stop(); + if (thread_.joinable()) { + thread_.join(); + } } -// NOLINTNEXTLINE(readability-static-accessed-through-instance) -InputReader::Task InputReader::read_input() { +void InputReader::read_input() { LOG_DEBUG("hidraw device: {}", device_); const UniqueFd fd(open(device_.c_str(), O_RDWR)); @@ -115,26 +134,87 @@ InputReader::Task InputReader::read_input() { os << CustomHexdump<400, false>(std::data(rpt_desc.value), rpt_desc.size); LOG_INFO(os.str()); + // Wait on both the hidraw fd and the stop eventfd so a blocking read can + // be interrupted immediately when stop() is called from another thread. + const UniqueFd epoll_fd(epoll_create1(EPOLL_CLOEXEC)); + if (!epoll_fd.valid()) { + LOG_ERROR("epoll_create1 failed: {}", strerror(errno)); + break; + } + epoll_event ev{}; + ev.events = EPOLLIN; + ev.data.fd = fd.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, fd.get(), &ev) == -1) { + LOG_ERROR("epoll_ctl(hidraw) failed: {}", strerror(errno)); + break; + } + if (stop_event_fd_.valid()) { + ev.data.fd = stop_event_fd_.get(); + if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, stop_event_fd_.get(), &ev) == + -1) { + LOG_ERROR("epoll_ctl(stop) failed: {}", strerror(errno)); + break; + } + } + while (!stop_flag_) { + std::array events{}; + const int nfds = + epoll_wait(epoll_fd.get(), events.data(), events.size(), -1); + if (nfds == -1) { + if (errno == EINTR) { + continue; + } + LOG_ERROR("epoll_wait failed: {}", strerror(errno)); + break; + } + + bool stop_requested = false; + bool data_ready = false; + for (int i = 0; i < nfds; ++i) { + if (stop_event_fd_.valid() && + events.at(i).data.fd == stop_event_fd_.get()) { + stop_requested = true; + } else if (events.at(i).data.fd == fd.get()) { + data_ready = true; + } + } + if (stop_requested) { + break; + } + if (!data_ready) { + continue; + } + std::array buffer{}; - ssize_t result = 0; - if (result = read(fd.get(), buffer.data(), buffer.size()); result < 0) { - LOG_ERROR("GetInputReport4 failed: {}", strerror(errno)); + const ssize_t result = read(fd.get(), buffer.data(), buffer.size()); + if (result < 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } + LOG_ERROR("read failed: {}", strerror(errno)); break; } + if (result == 0) { + continue; + } + const auto bytes_read = static_cast(result); if (raw_dev_info.product == 0x02FD) { if (const auto report_id = buffer.at(0); report_id == 1) { inputReport01_t input_report01{}; - std::memcpy(&input_report01, buffer.data(), sizeof(inputReport01_t)); + std::memcpy(&input_report01, buffer.data(), + std::min(sizeof(inputReport01_t), bytes_read)); PrintInputReport1(input_report01); } else if (report_id == 2) { inputReport02_t input_report02{}; - std::memcpy(&input_report02, buffer.data(), sizeof(inputReport02_t)); + std::memcpy(&input_report02, buffer.data(), + std::min(sizeof(inputReport02_t), bytes_read)); PrintInputReport2(input_report02); } else if (report_id == 4) { inputReport04_t input_report04{}; - std::memcpy(&input_report04, buffer.data(), sizeof(inputReport04_t)); + std::memcpy(&input_report04, buffer.data(), + std::min(sizeof(inputReport04_t), bytes_read)); PrintInputReport4(input_report04); } else { LOG_ERROR("Unknown report id: {}", report_id); @@ -144,10 +224,7 @@ InputReader::Task InputReader::read_input() { break; } - // fd is automatically closed by UniqueFd destructor - stop(); - - co_return; // NOLINT(readability-static-accessed-through-instance) + // fd is automatically closed by UniqueFd destructor. } std::string InputReader::dpad_to_string(const Direction dpad) { diff --git a/src/bluez/xbox_controller/input_reader.h b/src/bluez/xbox_controller/input_reader.h index 6ef06be..f519525 100644 --- a/src/bluez/xbox_controller/input_reader.h +++ b/src/bluez/xbox_controller/input_reader.h @@ -16,7 +16,7 @@ #define SRC_BLUEZ_XBOX_CONTROLLER_INPUT_READER_HPP_ #include -#include +#include #include "../hidraw.hpp" #include "xbox_controller_02fd.h" @@ -44,20 +44,16 @@ class InputReader { ~InputReader(); private: - struct Task { - struct promise_type { - static Task get_return_object() { return {}; } - static std::suspend_never initial_suspend() { return {}; } - static std::suspend_never final_suspend() noexcept { return {}; } - static void return_void() {} - static void unhandled_exception() { std::terminate(); } - }; - }; - std::string device_; std::atomic stop_flag_; - - Task read_input(); + // eventfd used to interrupt the blocking read loop immediately on stop(). + UniqueFd stop_event_fd_; + // Worker thread that owns the blocking read loop. Joined in the destructor + // before any other member is torn down, so the loop can never outlive this + // object (no use-after-free) and never blocks the D-Bus/main thread. + std::thread thread_; + + void read_input(); static std::string dpad_to_string(Direction dpad); diff --git a/src/bluez/xbox_controller/xbox_controller.cc b/src/bluez/xbox_controller/xbox_controller.cc index 155586f..827251c 100644 --- a/src/bluez/xbox_controller/xbox_controller.cc +++ b/src/bluez/xbox_controller/xbox_controller.cc @@ -46,6 +46,7 @@ XboxController::XboxController(sdbus::IConnection& connection) sub_system ? sub_system : ""); if (std::strcmp(sub_system, "hidraw") == 0) { if (std::strcmp(action, "remove") == 0) { + std::scoped_lock reader_lock(input_reader_mutex_); if (input_reader_) { input_reader_->stop(); input_reader_.reset(); @@ -157,6 +158,7 @@ void XboxController::onInterfacesAdded( if (const std::string hidraw_device = FindHidDevice(hidraw_device_key); !hidraw_device.empty()) { LOG_INFO("Adding hidraw device: {}", hidraw_device_key); + std::scoped_lock reader_lock(input_reader_mutex_); if (!input_reader_) { input_reader_ = std::make_unique(hidraw_device); input_reader_->start(); diff --git a/src/bluez/xbox_controller/xbox_controller.h b/src/bluez/xbox_controller/xbox_controller.h index 8159b25..0483e09 100644 --- a/src/bluez/xbox_controller/xbox_controller.h +++ b/src/bluez/xbox_controller/xbox_controller.h @@ -62,6 +62,10 @@ class XboxController final std::mutex upower_display_devices_mutex_; std::map> upower_clients_; + // Guards input_reader_, which is created/started from onInterfacesAdded + // (D-Bus event-loop or main thread) and stopped/reset from the udev monitor + // worker thread. + std::mutex input_reader_mutex_; std::unique_ptr input_reader_; void onInterfacesAdded( diff --git a/src/connman/connman_client.cc b/src/connman/connman_client.cc index f8830f6..0d4a07c 100644 --- a/src/connman/connman_client.cc +++ b/src/connman/connman_client.cc @@ -30,9 +30,16 @@ ConnmanManagerClient::ConnmanManagerClient(sdbus::IConnection& connection) registerProxy(); try { - // Load initial technologies + std::scoped_lock lock(maps_mutex_); + // Load initial technologies. Insert inline (rather than via + // onTechnologyAdded) so we don't re-acquire the non-recursive maps_mutex_. for (const auto& tech : GetTechnologies()) { - onTechnologyAdded(tech.get<0>(), tech.get<1>()); + const auto& path = tech.get<0>(); + LOG_INFO("Technology added: {}", path); + if (technologies_.find(path) == technologies_.end()) { + technologies_[path] = std::make_unique( + getProxy().getConnection(), path); + } } // Load initial services @@ -69,6 +76,7 @@ void ConnmanManagerClient::onTechnologyAdded( const std::map& properties) { (void)properties; try { + std::scoped_lock lock(maps_mutex_); LOG_INFO("Technology added: {}", path); if (technologies_.find(path) == technologies_.end()) { technologies_[path] = std::make_unique( @@ -81,6 +89,7 @@ void ConnmanManagerClient::onTechnologyAdded( void ConnmanManagerClient::onTechnologyRemoved(const sdbus::ObjectPath& path) { try { + std::scoped_lock lock(maps_mutex_); LOG_INFO("Technology removed: {}", path); technologies_.erase(path); } catch (const std::exception& e) { @@ -94,6 +103,7 @@ void ConnmanManagerClient::onServicesChanged( changed, const std::vector& removed) { try { + std::scoped_lock lock(maps_mutex_); for (const auto& service : changed) { const auto& path = service.get<0>(); const auto& props = service.get<1>(); diff --git a/src/connman/connman_client.h b/src/connman/connman_client.h index 2b39d5f..be1d09d 100644 --- a/src/connman/connman_client.h +++ b/src/connman/connman_client.h @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -62,6 +63,11 @@ class ConnmanManagerClient final const std::vector& removed) override; private: + // registerProxy() enables signal delivery on the async event-loop thread + // while the constructor still populates the maps on the calling thread, and + // onTechnology*/onServicesChanged mutate them from the event-loop thread. + // Guard all access with this mutex. + std::mutex maps_mutex_; std::map> technologies_; std::map> services_; diff --git a/src/geoclue2/main.cc b/src/geoclue2/main.cc index 308d7b4..4db855b 100644 --- a/src/geoclue2/main.cc +++ b/src/geoclue2/main.cc @@ -38,6 +38,11 @@ int main() { }); const auto& client = manager.Client(); + if (!client) { + LOG_ERROR("GeoClue2 did not return a client object; cannot continue"); + connection->leaveEventLoop(); + return 1; + } // `desktop id` must be set for Start to work client->DesktopId("org.example.geoclue2"); diff --git a/src/login1/login1_manager_client.cc b/src/login1/login1_manager_client.cc index ae0b84e..0829430 100644 --- a/src/login1/login1_manager_client.cc +++ b/src/login1/login1_manager_client.cc @@ -27,27 +27,36 @@ Login1ManagerClient::Login1ManagerClient(sdbus::IConnection& connection) if (!error) { onPropertiesChanged( sdbus::InterfaceName(Manager_proxy::INTERFACE_NAME), values, {}); - for (const auto seats = this->ListSeats(); - const auto& value : seats) { - // [('seat0', '/org/freedesktop/login1/seat/seat0')] - const auto& name = value.get<0>(); - const auto& path = value.get<1>(); - onSeatNew(name, path); - } - for (const auto sessions = this->ListSessions(); - const auto& value : sessions) { - // [('2', 1000, 'joel', 'seat0', - // '/org/freedesktop/login1/session/_32')] - const auto& user_name = value.get<2>(); - const auto& path = value.get<4>(); - onSessionNew(user_name, path); - } - for (const auto users = this->ListUsers(); - const auto& value : users) { - // [(1000, 'joel', '/org/freedesktop/login1/user/_1000')] - auto id = value.get<0>(); - const auto& path = value.get<2>(); - onUserNew(id, path); + // ListSeats/ListSessions/ListUsers are synchronous D-Bus calls; a + // throw here would escape the async reply handler into the event + // loop and terminate the process, so contain it. + try { + for (const auto seats = this->ListSeats(); + const auto& value : seats) { + // [('seat0', '/org/freedesktop/login1/seat/seat0')] + const auto& name = value.get<0>(); + const auto& path = value.get<1>(); + onSeatNew(name, path); + } + for (const auto sessions = this->ListSessions(); + const auto& value : sessions) { + // [('2', 1000, 'joel', 'seat0', + // '/org/freedesktop/login1/session/_32')] + const auto& user_name = value.get<2>(); + const auto& path = value.get<4>(); + onSessionNew(user_name, path); + } + for (const auto users = this->ListUsers(); + const auto& value : users) { + // [(1000, 'joel', '/org/freedesktop/login1/user/_1000')] + auto id = value.get<0>(); + const auto& path = value.get<2>(); + onUserNew(id, path); + } + } catch (const sdbus::Error& e) { + LOG_ERROR("[{}] enumerate failed: {} - {}", + Manager_proxy::INTERFACE_NAME, e.getName(), + e.getMessage()); } } else LOG_ERROR("[{}] {} - {}", Manager_proxy::INTERFACE_NAME, diff --git a/src/packagekit/main.cc b/src/packagekit/main.cc index e50354d..8153cf8 100644 --- a/src/packagekit/main.cc +++ b/src/packagekit/main.cc @@ -18,33 +18,43 @@ #include "../utils/utils.h" int main() { - const auto connection = sdbus::createSystemBusConnection(); - connection->enterEventLoopAsync(); - - { - PackageKitClient client(*connection); - std::promise> promise; - auto future = promise.get_future(); - - client.GetAllAsync( - PackageKitClient::INTERFACE_NAME, - [&](std::optional error, - std::map values) { - if (!error) { - promise.set_value(std::move(values)); - } else { - promise.set_exception(std::make_exception_ptr(std::move(*error))); - } - }); - - const auto properties = future.get(); - Utils::print_changed_properties( - sdbus::InterfaceName(PackageKitClient::INTERFACE_NAME), properties, {}); - auto state = client.GetDaemonState(); - LOG_INFO("Daemon {}", state); + try { + const auto connection = sdbus::createSystemBusConnection(); + connection->enterEventLoopAsync(); + + { + PackageKitClient client(*connection); + std::promise> promise; + auto future = promise.get_future(); + + client.GetAllAsync( + PackageKitClient::INTERFACE_NAME, + [&](std::optional error, + std::map values) { + if (!error) { + promise.set_value(std::move(values)); + } else { + promise.set_exception(std::make_exception_ptr(std::move(*error))); + } + }); + + const auto properties = future.get(); + Utils::print_changed_properties( + sdbus::InterfaceName(PackageKitClient::INTERFACE_NAME), properties, + {}); + auto state = client.GetDaemonState(); + LOG_INFO("Daemon {}", state); + } + + connection->leaveEventLoop(); + + return 0; + + } catch (const sdbus::Error& e) { + LOG_ERROR("D-Bus error: {} - {}", e.getName(), e.getMessage()); + return 1; + } catch (const std::exception& e) { + LOG_ERROR("Exception: {}", e.what()); + return 1; } - - connection->leaveEventLoop(); - - return 0; } diff --git a/src/systemd1/systemd1_manager_client.cc b/src/systemd1/systemd1_manager_client.cc index 3e93b3d..e5668ee 100644 --- a/src/systemd1/systemd1_manager_client.cc +++ b/src/systemd1/systemd1_manager_client.cc @@ -7,6 +7,7 @@ #include #include "../utils/logging.h" +#include "../utils/resource_limits.h" Systemd1ManagerClient::Systemd1ManagerClient(sdbus::IConnection& connection) : ProxyInterfaces{connection, sdbus::ServiceName(SERVICE_NAME), @@ -43,7 +44,17 @@ void Systemd1ManagerClient::onPropertiesChanged( void Systemd1ManagerClient::onUnitNew(const std::string& id, const sdbus::ObjectPath& unit) { - activeUnits_.push_back(id); + // systemd emits UnitNew for thousands of units and re-emits them over a long + // run; de-duplicate and cap growth so activeUnits_ cannot grow unbounded. + if (std::ranges::find(activeUnits_, id) == activeUnits_.end()) { + if (resource_limits::IsAtCapacity(activeUnits_.size(), + resource_limits::kMaxUnits)) { + LOG_WARN("[systemd1] Skipping UnitNew id={}: resource limit reached ({})", + id, resource_limits::kMaxUnits); + } else { + activeUnits_.push_back(id); + } + } LOG_INFO("[systemd1] UnitNew id={} path={}", id, static_cast(unit)); } diff --git a/src/timedate1/timedate1_client.cc b/src/timedate1/timedate1_client.cc index cc8cacd..069c6bc 100644 --- a/src/timedate1/timedate1_client.cc +++ b/src/timedate1/timedate1_client.cc @@ -63,8 +63,11 @@ void appendTimeUSecAsDate(const uint64_t timeUSec, std::ostringstream& ss) { std::chrono::microseconds(timeUSec)); const std::time_t timeT = std::chrono::system_clock::to_time_t(timePoint); std::tm tm_buf{}; - const std::tm* tm = localtime_r(&timeT, &tm_buf); - ss << "\tDate: " << std::put_time(tm, "%d-%m-%Y %H:%M:%S") << std::endl; + if (const std::tm* tm = localtime_r(&timeT, &tm_buf); tm != nullptr) { + ss << "\tDate: " << std::put_time(tm, "%d-%m-%Y %H:%M:%S") << std::endl; + } else { + ss << "\tDate: (invalid)" << std::endl; + } } void Timedate1Client::printTimedate1() const { diff --git a/src/udisks2/udisks2_manager.cc b/src/udisks2/udisks2_manager.cc index 582409e..951ed26 100644 --- a/src/udisks2/udisks2_manager.cc +++ b/src/udisks2/udisks2_manager.cc @@ -53,6 +53,7 @@ void UDisks2Manager::onInterfacesAdded( continue; } if ("org.freedesktop.UDisks2.Manager.NVMe" == interface) { + std::lock_guard lock(manager_nvme_mutex_); manager_nvme_ = std::make_unique( getProxy().getConnection(), objectPath); } else if ("org.freedesktop.UDisks2.Block" == interface) { @@ -115,6 +116,7 @@ void UDisks2Manager::onInterfacesRemoved( for (const auto& interface : interfaces) { LOG_INFO("[{}] Remove - {}", objectPath, interface); if ("org.freedesktop.UDisks2.Manager.NVMe" == interface) { + std::lock_guard lock(manager_nvme_mutex_); manager_nvme_.reset(); } else if ("org.freedesktop.UDisks2.Block" == interface) { std::lock_guard lock(block_mutex_); diff --git a/src/udisks2/udisks2_manager.h b/src/udisks2/udisks2_manager.h index 56a8d3c..33a3b5f 100644 --- a/src/udisks2/udisks2_manager.h +++ b/src/udisks2/udisks2_manager.h @@ -38,6 +38,7 @@ class UDisks2Manager final static constexpr auto OBJECT_PATH = "/org/freedesktop/UDisks2"; std::unique_ptr manager_nvme_; + std::mutex manager_nvme_mutex_; std::map> blocks_; std::mutex block_mutex_; diff --git a/src/upower/upower_display_device.cc b/src/upower/upower_display_device.cc index 543bf3d..02bf902 100644 --- a/src/upower/upower_display_device.cc +++ b/src/upower/upower_display_device.cc @@ -41,146 +41,80 @@ void UPowerDisplayDevice::onPropertiesChanged( const sdbus::InterfaceName& interfaceName, const std::map& changedProperties, const std::vector& invalidatedProperties) { - if (const auto key = sdbus::MemberName("BatteryLevel"); - changedProperties.contains(key)) { - properties_.battery_level = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Capacity"); - changedProperties.contains(key)) { - properties_.capacity = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ChargeCycles"); - changedProperties.contains(key)) { - properties_.charge_cycles = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ChargeEndThreshold"); - changedProperties.contains(key)) { - properties_.charge_end_threshold = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ChargeStartThreshold"); - changedProperties.contains(key)) { - properties_.charge_start_threshold = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ChargeThresholdEnabled"); - changedProperties.contains(key)) { - properties_.charge_threshold_enabled = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("ChargeThresholdSupported"); - changedProperties.contains(key)) { - properties_.charge_threshold_supported = - changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Energy"); - changedProperties.contains(key)) { - properties_.energy = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("EnergyEmpty"); - changedProperties.contains(key)) { - properties_.energy_empty = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("EnergyFull"); - changedProperties.contains(key)) { - properties_.energy_full = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("EnergyFullDesign"); - changedProperties.contains(key)) { - properties_.energy_full_design = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("EnergyRate"); - changedProperties.contains(key)) { - properties_.energy_rate = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("HasHistory"); - changedProperties.contains(key)) { - properties_.has_history = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("HasStatistics"); - changedProperties.contains(key)) { - properties_.has_statistics = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("IconName"); - changedProperties.contains(key)) { - properties_.icon_name = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("IsPresent"); - changedProperties.contains(key)) { - properties_.is_present = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("IsRechargeable"); - changedProperties.contains(key)) { - properties_.is_rechargeable = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Luminosity"); - changedProperties.contains(key)) { - properties_.luminosity = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Model"); - changedProperties.contains(key)) { - properties_.model = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("NativePath"); - changedProperties.contains(key)) { - properties_.native_path = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Online"); - changedProperties.contains(key)) { - properties_.online = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Percentage"); - changedProperties.contains(key)) { - properties_.percentage = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("PowerSupply"); - changedProperties.contains(key)) { - properties_.power_supply = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Serial"); - changedProperties.contains(key)) { - properties_.serial = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("State"); - changedProperties.contains(key)) { - properties_.state = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Technology"); - changedProperties.contains(key)) { - properties_.technology = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Temperature"); - changedProperties.contains(key)) { - properties_.temperature = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("TimeToEmpty"); - changedProperties.contains(key)) { - properties_.time_to_empty = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("TimeToFull"); - changedProperties.contains(key)) { - properties_.time_to_full = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Type"); - changedProperties.contains(key)) { - properties_.type = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("UpdateTime"); - changedProperties.contains(key)) { - properties_.update_time = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Vendor"); - changedProperties.contains(key)) { - properties_.vendor = changedProperties.at(key).get(); - } - if (const auto key = sdbus::MemberName("Voltage"); - changedProperties.contains(key)) { - properties_.voltage = changedProperties.at(key).get(); - } - - if (const auto key = sdbus::MemberName("WarningLevel"); - changedProperties.contains(key)) { - properties_.warning_level = changedProperties.at(key).get(); + // Iterate the changed properties once and dispatch on the key. A + // PropertiesChanged signal usually carries only a handful of properties, + // so this avoids scanning every known property (and the per-property map + // lookup + MemberName allocation) on every signal. + for (const auto& [key, value] : changedProperties) { + if (key == "BatteryLevel") { + properties_.battery_level = value.get(); + } else if (key == "Capacity") { + properties_.capacity = value.get(); + } else if (key == "ChargeCycles") { + properties_.charge_cycles = value.get(); + } else if (key == "ChargeEndThreshold") { + properties_.charge_end_threshold = value.get(); + } else if (key == "ChargeStartThreshold") { + properties_.charge_start_threshold = value.get(); + } else if (key == "ChargeThresholdEnabled") { + properties_.charge_threshold_enabled = value.get(); + } else if (key == "ChargeThresholdSupported") { + properties_.charge_threshold_supported = value.get(); + } else if (key == "Energy") { + properties_.energy = value.get(); + } else if (key == "EnergyEmpty") { + properties_.energy_empty = value.get(); + } else if (key == "EnergyFull") { + properties_.energy_full = value.get(); + } else if (key == "EnergyFullDesign") { + properties_.energy_full_design = value.get(); + } else if (key == "EnergyRate") { + properties_.energy_rate = value.get(); + } else if (key == "HasHistory") { + properties_.has_history = value.get(); + } else if (key == "HasStatistics") { + properties_.has_statistics = value.get(); + } else if (key == "IconName") { + properties_.icon_name = value.get(); + } else if (key == "IsPresent") { + properties_.is_present = value.get(); + } else if (key == "IsRechargeable") { + properties_.is_rechargeable = value.get(); + } else if (key == "Luminosity") { + properties_.luminosity = value.get(); + } else if (key == "Model") { + properties_.model = value.get(); + } else if (key == "NativePath") { + properties_.native_path = value.get(); + } else if (key == "Online") { + properties_.online = value.get(); + } else if (key == "Percentage") { + properties_.percentage = value.get(); + } else if (key == "PowerSupply") { + properties_.power_supply = value.get(); + } else if (key == "Serial") { + properties_.serial = value.get(); + } else if (key == "State") { + properties_.state = value.get(); + } else if (key == "Technology") { + properties_.technology = value.get(); + } else if (key == "Temperature") { + properties_.temperature = value.get(); + } else if (key == "TimeToEmpty") { + properties_.time_to_empty = value.get(); + } else if (key == "TimeToFull") { + properties_.time_to_full = value.get(); + } else if (key == "Type") { + properties_.type = value.get(); + } else if (key == "UpdateTime") { + properties_.update_time = value.get(); + } else if (key == "Vendor") { + properties_.vendor = value.get(); + } else if (key == "Voltage") { + properties_.voltage = value.get(); + } else if (key == "WarningLevel") { + properties_.warning_level = value.get(); + } } Utils::print_changed_properties(interfaceName, changedProperties, invalidatedProperties); diff --git a/src/utils/resource_limits.h b/src/utils/resource_limits.h index 6f6505e..ac1be3f 100644 --- a/src/utils/resource_limits.h +++ b/src/utils/resource_limits.h @@ -27,6 +27,7 @@ inline constexpr std::size_t kMaxGattDescriptors = 32768; inline constexpr std::size_t kMaxBatteryEntries = 1024; inline constexpr std::size_t kMaxInputEntries = 1024; inline constexpr std::size_t kMaxUPowerClients = 256; +inline constexpr std::size_t kMaxUnits = 16384; inline bool IsAtCapacity(const std::size_t current_size, const std::size_t limit) { diff --git a/src/utils/utils.cc b/src/utils/utils.cc index a907829..6a998b0 100644 --- a/src/utils/utils.cc +++ b/src/utils/utils.cc @@ -131,10 +131,8 @@ const std::unordered_map< }}, {"s", [](const sdbus::Variant& v, std::ostringstream& os) { - os << (v.get().empty() - ? "\"\"" - : v.get()) - << std::endl; + const auto s = v.get(); + os << (s.empty() ? "\"\"" : s) << std::endl; }}, {"x", [](const sdbus::Variant& v, std::ostringstream& os) { @@ -148,62 +146,64 @@ const std::unordered_map< }}, {"as", [](const sdbus::Variant& v, std::ostringstream& os) { + const auto vec = v.get>(); os << std::endl; - for (const auto& s : - v.get>()) { + for (const auto& s : vec) { os << "\t" << s << std::endl; } - if (v.get>().empty()) + if (vec.empty()) os << "\t" << "\"\"" << std::endl; }}, {"au", [](const sdbus::Variant& v, std::ostringstream& os) { + const auto vec = v.get>(); os << std::endl; - for (const auto& s : - v.get>()) { + for (const auto& s : vec) { os << "\t" << std::to_string(s) << std::endl; } - if (v.get>().empty()) + if (vec.empty()) os << "\t" << "\"\"" << std::endl; else os << std::endl; }}, {"aau", [](const sdbus::Variant& v, std::ostringstream& os) { + const auto vec = + v.get>>(); os << std::endl; - for (const auto& it : - v.get>>()) { + for (const auto& it : vec) { for (const auto& address : it) { os << "\t" << std::to_string(address) << std::endl; } } - if (v.get>>() - .empty()) + if (vec.empty()) os << "\t" << "\"\"" << std::endl; else os << std::endl; }}, {"ay", [](const sdbus::Variant& v, std::ostringstream& os) { - for (const auto& b : v.get>()) { + const auto vec = v.get>(); + for (const auto& b : vec) { os << std::hex << std::setw(2) << std::setfill('0') << static_cast(b) << " "; } - if (v.get>().empty()) + if (vec.empty()) os << "\t" << "\"\"" << std::endl; else os << std::endl; }}, {"aay", [](const sdbus::Variant& v, std::ostringstream& os) { - for (const auto& it : - v.get>>()) { + const auto vec = + v.get>>(); + for (const auto& it : vec) { for (const auto& b : it) { os << std::hex << std::setw(2) << std::setfill('0') << static_cast(b) << " "; } } - if (v.get>>().empty()) + if (vec.empty()) os << "\t" << "\"\"" << std::endl; else os << std::endl;