SimpleBLE icon indicating copy to clipboard operation
SimpleBLE copied to clipboard

removing on_value_changed callbacks can throw exceptions leading to things not being freed correctly

Open jcelerier opened this issue 1 year ago • 0 comments

In void PeripheralBase::_cleanup_characteristics() noexcept :

https://github.com/OpenBluetoothToolbox/SimpleBLE/blob/0ec39251445c63d572d4406902231efd99841a64/simpleble/src/backends/linux/PeripheralLinux.cpp#L307

Here:

    frame #0: 0x00007ffff24b0780 libstdc++.so.6`__cxxabiv1::__cxa_throw(obj=0x0000555557107d30, tinfo=0x00007fffcc77c4d8, dest=(libsimpleble-debug.so.0`SimpleDBus::Exception::InterfaceNotFoundException::~InterfaceNotFoundException() at Exceptions.h:36)) at eh_throw.cc:80:1
  * frame #1: 0x00007fffcc7514e3 libsimpleble-debug.so.0`SimpleDBus::Proxy::interface_get(this=0x000055555704fe30, name="org.bluez.GattCharacteristic1") at Proxy.cpp:50:9
    frame #2: 0x00007fffcc720225 libsimpleble-debug.so.0`SimpleBluez::Characteristic::gattcharacteristic1(this=0x000055555704fe30) at Characteristic.cpp:30:59
    frame #3: 0x00007fffcc720e9a libsimpleble-debug.so.0`SimpleBluez::Characteristic::clear_on_value_changed(this=0x000055555704fe30) at Characteristic.cpp:73:49
    frame #4: 0x00007fffcc703ec0 libsimpleble-debug.so.0`SimpleBLE::PeripheralBase::_cleanup_characteristics(this=0x00007fff2c1cc5b0) at PeripheralBase.cpp:309:39
    frame #5: 0x00007fffcc703c31 libsimpleble-debug.so.0`SimpleBLE::PeripheralBase::~PeripheralBase(this=0x00007fff2c1cc5b0) at PeripheralBase.cpp:32:5
    frame #6: 0x00007fffcc7019c5 libsimpleble-debug.so.0`void std::destroy_at<SimpleBLE::PeripheralBase>(__location=0x00007fff2c1cc5b0) at stl_construct.h:88:15

I see exceptions being thrown for for instance org.bluez.GattCharacteristic1, thus

        for (auto bluez_service : device_->services()) {
            for (auto bluez_characteristic : bluez_service->characteristics()) {
                bluez_characteristic->clear_on_value_changed();
            }
        }

if there are multiple services & characteristics, all the following ones will not be cleared :/ I think the catch clause should be inside the innermost for loop, just like for the subsequent loop:

        // Stop notifying all characteristics.
        for (auto bluez_service : device_->services()) {
            for (auto bluez_characteristic : bluez_service->characteristics()) {
                try {
                    if (bluez_characteristic->notifying()) {
                        bluez_characteristic->stop_notify();
                    }
                } catch (std::exception const& e) {
                    SIMPLEBLE_LOG_WARN(fmt::format("Exception during characteristic cleanup: {}", e.what()));
                }
            }
        }

jcelerier avatar Dec 13 '24 15:12 jcelerier