Migrated to new message forwarding logic on SimpleDBus. Fixed race condition.

This commit is contained in:
Kevin Dewald
2025-08-03 00:24:02 -07:00
parent 870ae77cfd
commit d1c19790cd
9 changed files with 89 additions and 97 deletions

View File

@@ -16,21 +16,23 @@ The format is based on `Keep a Changelog`_, and this project adheres to `Semanti
**Added**
- Introduced scaffolding for advanced low-level features.
- Configuration option to select which BlueZ backend to use.
- (Android) Added support to set and retrieve the JavaVM pointer.
- (Linux) Added frozen BlueZ backend in preparation for upcoming changes.
- Configuration option to select which BlueZ backend to use.
**Changed**
- (MacOS) Use a single Adapter object across all users of the CoreBluetooth backend.
- (Android) Use a single Adapter object across all users of the Android backend.
- (SimpleDBus) Messages are now directly forwarded to the appropriate proxy object, no more chaining required.
**Fixed**
- Improper handling of configuration settings when consuming SimpleBLE as a shared library.
- (MacOS) Freeze when attempting a double disconnection.
- (Android) Solved "local reference table overflow" error. *(Thanks Nicole S.!)*
- (Android) Fixed unexpected initialization of SimpleJNI.
- Improper handling of configuration settings when consuming SimpleBLE as a shared library.
- (SimpleDBus) Fixed race condition when handling property updates of DBus objects.
**Removed**

View File

@@ -30,7 +30,7 @@ int main() {
adapter.set_callback_on_scan_stop([]() { std::cout << "Scan stopped." << std::endl; });
// Scan for 5 seconds.
adapter.scan_for(5000);
adapter.scan_for(15000);
std::cout << "Scan complete." << std::endl;

View File

@@ -3,7 +3,7 @@
namespace SimpleBLE {
namespace Config {
namespace SimpleBluez {
bool use_legacy_bluez_backend = true;
bool use_legacy_bluez_backend = false;
std::chrono::steady_clock::duration connection_timeout = std::chrono::seconds(2);
std::chrono::steady_clock::duration disconnection_timeout = std::chrono::seconds(1);
} // namespace SimpleBluez

View File

@@ -26,16 +26,7 @@ void Bluez::init() {
}
void Bluez::run_async() {
// TODO: UNCOMMENT THIS WHEN MIGRATING TO NEW PROXY FORWARDING LOGIC
//_conn->read_write_dispatch();
// BELOW IS THE LEGACY LOGIC
_conn->read_write();
SimpleDBus::Message message = _conn->pop_message();
while (message.is_valid()) {
_bluez_root->message_forward(message);
message = _conn->pop_message();
}
_conn->read_write_dispatch();
}
std::vector<std::shared_ptr<Adapter>> Bluez::get_adapters() { return _bluez_root->get_adapters(); }

View File

@@ -66,7 +66,6 @@ class Proxy {
void path_append_child(const std::string& path, std::shared_ptr<Proxy> child);
void path_remove_child(const std::string& path);
// ----- MESSAGE HANDLING -----
void message_forward(Message& msg);
void message_handle(Message& msg);
// ----- CALLBACKS -----

View File

@@ -22,10 +22,7 @@ class Connection {
void add_match(std::string rule);
void remove_match(std::string rule);
void read_write();
void read_write_dispatch();
Message pop_message();
void send(Message& msg);
Message send_with_reply_and_block(Message& msg);

View File

@@ -104,22 +104,25 @@ void Interface::property_refresh(const std::string& property_name) {
}
bool cb_property_changed_required = false;
_property_update_mutex.lock();
try {
// NOTE: Due to the way Bluez handles underlying devices and the fact that
// they can be removed before the callback reaches back (race condition),
// `property_get` can sometimes fail. Because of this, the update
// statement is surrounded by a try-catch statement.
Holder property_latest = property_get(property_name);
_property_update_mutex.lock();
_property_valid_map[property_name] = true;
if (_properties[property_name] != property_latest) {
_properties[property_name] = property_latest;
cb_property_changed_required = true;
}
_property_update_mutex.unlock();
} catch (const Exception::SendFailed& e) {
_property_update_mutex.lock();
_property_valid_map[property_name] = true;
_property_update_mutex.unlock();
}
_property_update_mutex.unlock();
if (cb_property_changed_required) {
property_changed(property_name);

View File

@@ -5,6 +5,7 @@
#include <simpledbus/base/Logging.h>
#include <simpledbus/base/Path.h>
#include <algorithm>
#include <iostream>
// #include <simpledbus/interfaces/Properties.h>
@@ -12,16 +13,14 @@ using namespace SimpleDBus;
Proxy::Proxy(std::shared_ptr<Connection> conn, const std::string& bus_name, const std::string& path)
: _conn(conn), _bus_name(bus_name), _path(path), _valid(true), _registered(false) {
// TODO: UNCOMMENT THIS WHEN MIGRATING TO NEW PROXY FORWARDING LOGIC
// register_object_path();
register_object_path();
//_interfaces.emplace(std::make_pair("org.freedesktop.DBus.Properties", std::make_shared<Properties>(conn, bus_name,
//path)));
// TODO: At some point in the future, proxy objects will own their own Properties interface.
//_interfaces.emplace(std::make_pair("org.freedesktop.DBus.Properties", std::make_shared<Properties>(conn, bus_name, path)));
}
Proxy::~Proxy() {
// TODO: UNCOMMENT THIS WHEN MIGRATING TO NEW PROXY FORWARDING LOGIC
// unregister_object_path();
unregister_object_path();
on_child_created.unload();
on_signal_received.unload();
}
@@ -34,8 +33,7 @@ bool Proxy::valid() const { return _valid; }
void Proxy::invalidate() {
_valid = false;
// TODO: UNCOMMENT THIS WHEN MIGRATING TO NEW PROXY FORWARDING LOGIC
// unregister_object_path();
unregister_object_path();
}
std::string Proxy::path() const { return _path; }
@@ -314,52 +312,79 @@ void Proxy::path_remove_child(const std::string& path) {
// ----- MESSAGE HANDLING -----
void Proxy::message_forward(Message& msg) {
// If the message is for the current proxy, then forward it to the message handler.
if (msg.get_path() == _path) {
// If the message is involves a property change, forward it to the correct interface.
if (msg.is_signal("org.freedesktop.DBus.Properties", "PropertiesChanged")) {
Holder interface_h = msg.extract();
std::string iface_name = interface_h.get_string();
msg.extract_next();
Holder changed_properties = msg.extract();
msg.extract_next();
Holder invalidated_properties = msg.extract();
// If the interface is not loaded, then ignore the message.
if (!interface_exists(iface_name)) {
return;
}
interface_get(iface_name)->signal_property_changed(changed_properties, invalidated_properties);
} else if (interface_exists(msg.get_interface())) {
interface_get(msg.get_interface())->message_handle(msg);
}
return;
}
// If the message is for a child proxy or a descendant, forward it to that child proxy.
for (auto& [child_path, child] : _children) {
if (child_path == msg.get_path()) {
child->message_forward(msg);
if (msg.get_type() == Message::Type::SIGNAL) {
child->on_signal_received();
}
return;
} else if (PathUtils::is_descendant(child_path, msg.get_path())) {
child->message_forward(msg);
return;
}
}
}
void Proxy::message_handle(Message& msg) {
bool handled = false;
// ! The following block emulates the presence of a Properties interface.
// TODO: Remove this block when the Properties interface is implemented.
const std::string properties_interface_name = "org.freedesktop.DBus.Properties";
if (msg.is_method_call(properties_interface_name, "GetAll")) {
Holder interface_h = msg.extract();
std::string iface_name = interface_h.get_string();
// TODO: Handle the case where the interface does not exist, returning an error.
std::shared_ptr<Interface> interface = interface_get(iface_name);
Holder result = interface->property_collect();
SimpleDBus::Message reply = SimpleDBus::Message::create_method_return(msg);
reply.append_argument(result, "a{sv}");
_conn->send(reply);
handled = true;
} else if (msg.is_method_call(properties_interface_name, "Get")) {
Holder interface_h = msg.extract();
std::string iface_name = interface_h.get_string();
msg.extract_next();
Holder property_h = msg.extract();
std::string property_name = property_h.get_string();
// TODO: Handle the case where the interface does not exist, returning an error.
std::shared_ptr<Interface> interface = interface_get(iface_name);
Holder result = interface->property_collect_single(property_name);
SimpleDBus::Message reply = SimpleDBus::Message::create_method_return(msg);
reply.append_argument(result, "v");
_conn->send(reply);
handled = true;
} else if (msg.is_method_call(properties_interface_name, "Set")) {
Holder interface_h = msg.extract();
std::string iface_name = interface_h.get_string();
msg.extract_next();
Holder property_h = msg.extract();
std::string property_name = property_h.get_string();
msg.extract_next();
Holder value_h = msg.extract();
// TODO: Handle the case where the interface does not exist, returning an error.
std::shared_ptr<Interface> interface = interface_get(iface_name);
interface->property_modify(property_name, value_h);
SimpleDBus::Message reply = SimpleDBus::Message::create_method_return(msg);
_conn->send(reply);
handled = true;
} else if (msg.is_signal(properties_interface_name, "PropertiesChanged")) {
Holder interface_h = msg.extract();
std::string iface_name = interface_h.get_string();
msg.extract_next();
Holder changed_properties = msg.extract();
msg.extract_next();
Holder invalidated_properties = msg.extract();
if (interface_exists(iface_name)) {
interface_get(iface_name)->signal_property_changed(changed_properties, invalidated_properties);
handled = true;
}
} else
// ! This is the only block that should be used to forward messages to interfaces.
if (interface_exists(msg.get_interface())) {
interface_get(msg.get_interface())->message_handle(msg);
handled = true;

View File

@@ -49,8 +49,7 @@ void Connection::uninit() {
SimpleDBus::Message message;
do {
std::this_thread::sleep_for(std::chrono::milliseconds(10));
read_write();
message = pop_message();
read_write_dispatch();
} while (message.is_valid());
dbus_connection_unref(_conn);
@@ -99,18 +98,6 @@ void Connection::remove_match(std::string rule) {
}
}
void Connection::read_write() {
// TODO: DEPRECATE
if (!_initialized) {
throw Exception::NotInitialized();
}
std::lock_guard<std::recursive_mutex> lock(_mutex);
// Non blocking read of the next available message
dbus_connection_read_write(_conn, 0);
}
void Connection::read_write_dispatch() {
if (!_initialized) {
throw Exception::NotInitialized();
@@ -125,18 +112,6 @@ void Connection::read_write_dispatch() {
while (dbus_connection_dispatch(_conn) == DBUS_DISPATCH_DATA_REMAINS) {}
}
Message Connection::pop_message() {
// TODO: DEPRECATE
if (!_initialized) {
throw Exception::NotInitialized();
}
std::lock_guard<std::recursive_mutex> lock(_mutex);
DBusMessage* msg = dbus_connection_pop_message(_conn);
return Message::from_acquired(msg);
}
void Connection::send(Message& msg) {
if (!_initialized) {
throw Exception::NotInitialized();