Resolve circular references (p1): C++ GATTHandler, GATTService, pp are not owner of their resepctive backreference

GATTHandler, GATTService, pp are not owner of their resepctive backreference,
hence use std::weak_ptr for backreferences in general and validate on usage (nullptr, if destructed).

No DBTDevice has been ever destructed after using GATTHandler and discovering all GATT services.
In contrast to Java, C++ has no magic GC and hence shared_ptr use_count gets only increased
when emplying circular backreferences - none gets destructed.

Current ownership relationship is:
  DBTAdapter -> DBTDevice -> GATTHandler -> GATTService ...

each contains a backreference, now using a weak_ptr.

Result is that depending on the use-case, DBTDevice instances are destructed:
- Using device->remove(): Immediately
- No explicit device->remove(): Adapter keeps sharedDevices, destruction occurs at end.

5 files changed, 62 insertions, 18 deletions

diff --git a/src/direct_bt/DBTAdapter.cpp b/src/direct_bt/DBTAdapter.cpp
index ecab015a..d8143042 100644
--- a/src/direct_bt/DBTAdapter.cpp
+++ b/src/direct_bt/DBTAdapter.cpp
@@ -196,6 +196,16 @@ DBTAdapter::~DBTAdapter() {
     DBG_PRINT("DBTAdapter::dtor: XXX");
 }
 
+void DBTAdapter::printDevices() {
+    const std::lock_guard<std::recursive_mutex> lock0(mtx_connectedDevices);
+    const std::lock_guard<std::recursive_mutex> lock1(mtx_discoveredDevices);
+    const std::lock_guard<std::recursive_mutex> lock2(mtx_sharedDevices);
+
+    printSharedPtrList("SharedDevices", sharedDevices);
+    printSharedPtrList("DiscoveredDevices", discoveredDevices);
+    printSharedPtrList("ConnectedDevices", connectedDevices);
+}
+
 std::shared_ptr<NameAndShortName> DBTAdapter::setLocalName(const std::string &name, const std::string &short_name) {
     return mgmt.setLocalName(dev_id, name, short_name);
 }
diff --git a/src/direct_bt/DBTDevice.cpp b/src/direct_bt/DBTDevice.cpp
index 02412b49..14fefaab 100644
--- a/src/direct_bt/DBTDevice.cpp
+++ b/src/direct_bt/DBTDevice.cpp
@@ -59,10 +59,11 @@ DBTDevice::DBTDevice(DBTAdapter & a, EInfoReport const & r)
 }
 
 DBTDevice::~DBTDevice() {
-    DBG_PRINT("DBTDevice::dtor: ... %p %s", this, toString().c_str());
+    DBG_PRINT("DBTDevice::dtor: ... %p %s", this, getAddressString().c_str());
     remove();
     services.clear();
     msd = nullptr;
+    DBG_PRINT("DBTDevice::dtor: XXX %p %s", this, getAddressString().c_str());
 }
 
 std::shared_ptr<DBTDevice> DBTDevice::getSharedInstance() const {
@@ -398,7 +399,7 @@ exit:
 }
 
 void DBTDevice::remove() {
-    disconnect();
+    disconnect(false /* fromDisconnectCB */, false /* ioErrorCause */, HCIStatusCode::REMOTE_USER_TERMINATED_CONNECTION);
     adapter.removeConnectedDevice(*this); // usually done in DBTAdapter::mgmtEvDeviceDisconnectedHCI
     adapter.removeDiscoveredDevice(*this); // usually done in DBTAdapter::mgmtEvDeviceDisconnectedHCI
     releaseSharedInstance();
diff --git a/src/direct_bt/GATTCharacteristic.cpp b/src/direct_bt/GATTCharacteristic.cpp
index ebb88d2e..c6de3092 100644
--- a/src/direct_bt/GATTCharacteristic.cpp
+++ b/src/direct_bt/GATTCharacteristic.cpp
@@ -39,10 +39,6 @@
 
 using namespace direct_bt;
 
-std::shared_ptr<DBTDevice> GATTCharacteristic::getDevice() {
-    return service->device;
-}
-
 #define CHAR_DECL_PROPS_ENUM(X) \
         X(Broadcast,broadcast) \
         X(Read,read) \
@@ -67,6 +63,14 @@ std::shared_ptr<DBTDevice> GATTCharacteristic::getDevice() {
 
 #define CASE_TO_STRING2(V,S) case V: return #S;
 
+std::shared_ptr<DBTDevice> GATTCharacteristic::getDevice() const {
+    std::shared_ptr<GATTService> s = getService();
+    if( nullptr != s ) {
+        return s->getDevice();
+    }
+    return nullptr;
+}
+
 std::string GATTCharacteristic::getPropertyString(const PropertyBitVal prop) {
     switch(prop) {
         CHAR_DECL_PROPS_ENUM(CASE_TO_STRING2)
@@ -106,14 +110,23 @@ std::vector<std::unique_ptr<std::string>> GATTCharacteristic::getPropertiesStrin
 }
 
 std::string GATTCharacteristic::toString() const {
-    const std::shared_ptr<const uuid_t> & service_uuid = service->type;
-    const uint16_t service_handle_end = service->endHandle;
+    std::shared_ptr<const uuid_t> service_uuid;
+    uint16_t service_handle_end = 0xffff;
+    GATTServiceRef serviceRef = getService();
+    std::string service_uuid_str = "";
     std::string service_name = "";
     std::string char_name = "";
     std::string desc_str = ", descr[ ";
-    if( uuid_t::UUID16_SZ == service_uuid->getTypeSize() ) {
-        const uint16_t uuid16 = (static_cast<const uuid16_t*>(service_uuid.get()))->value;
-        service_name = ", "+GattServiceTypeToString(static_cast<GattServiceType>(uuid16));
+
+    if( nullptr != serviceRef ) {
+        service_uuid = serviceRef->type;
+        service_uuid_str = service_uuid->toString();
+        service_handle_end = serviceRef->endHandle;
+
+        if( uuid_t::UUID16_SZ == service_uuid->getTypeSize() ) {
+            const uint16_t uuid16 = (static_cast<const uuid16_t*>(service_uuid.get()))->value;
+            service_name = ", "+GattServiceTypeToString(static_cast<GattServiceType>(uuid16));
+        }
     }
     if( uuid_t::UUID16_SZ == value_type->getTypeSize() ) {
         const uint16_t uuid16 = (static_cast<const uuid16_t*>(value_type.get()))->value;
@@ -126,7 +139,7 @@ std::string GATTCharacteristic::toString() const {
     desc_str += " ]";
     return "handle "+uint16HexString(handle)+", props "+uint8HexString(properties)+" "+getPropertiesString()+
            ", value[type 0x"+value_type->toString()+", handle "+uint16HexString(value_handle)+char_name+desc_str+
-           "], service[type 0x"+service_uuid->toString()+
+           "], service[type 0x"+service_uuid_str+
            ", handle[ "+uint16HexString(service_handle)+".."+uint16HexString(service_handle_end)+" ]"+
            service_name+" ]";
 }
diff --git a/src/direct_bt/GATTDescriptor.cpp b/src/direct_bt/GATTDescriptor.cpp
index 66be0961..5d23c97f 100644
--- a/src/direct_bt/GATTDescriptor.cpp
+++ b/src/direct_bt/GATTDescriptor.cpp
@@ -43,12 +43,19 @@ const uuid16_t GATTDescriptor::TYPE_EXT_PROP(Type::CHARACTERISTIC_EXTENDED_PROPE
 const uuid16_t GATTDescriptor::TYPE_USER_DESC(Type::CHARACTERISTIC_USER_DESCRIPTION);
 const uuid16_t GATTDescriptor::TYPE_CCC_DESC(Type::CLIENT_CHARACTERISTIC_CONFIGURATION);
 
-std::shared_ptr<DBTDevice> GATTDescriptor::getDevice() {
-    return characteristic->service->device;
+std::shared_ptr<DBTDevice> GATTDescriptor::getDevice() const {
+    std::shared_ptr<GATTCharacteristic> c = getCharacteristic();
+    if( nullptr != c ) {
+        return c->getDevice();
+    }
+    return nullptr;
 }
 
 bool GATTDescriptor::readValue(int expectedLength) {
     std::shared_ptr<DBTDevice> device = getDevice();
+    if( nullptr == device ) {
+        throw IllegalStateException("Descriptor's device already destructed: "+toString(), E_FILE_LINE);
+    }
     std::shared_ptr<GATTHandler> gatt = device->getGATTHandler();
     if( nullptr == gatt ) {
         throw IllegalStateException("Descriptor's device GATTHandle not connected: "+
@@ -59,6 +66,9 @@ bool GATTDescriptor::readValue(int expectedLength) {
 
 bool GATTDescriptor::writeValue() {
     std::shared_ptr<DBTDevice> device = getDevice();
+    if( nullptr == device ) {
+        throw IllegalStateException("Descriptor's device already destructed: "+toString(), E_FILE_LINE);
+    }
     std::shared_ptr<GATTHandler> gatt = device->getGATTHandler();
     if( nullptr == gatt ) {
         throw IllegalStateException("Descriptor's device GATTHandle not connected: "+
diff --git a/src/direct_bt/GATTHandler.cpp b/src/direct_bt/GATTHandler.cpp
index 2eb45f7c..ae33af5c 100644
--- a/src/direct_bt/GATTHandler.cpp
+++ b/src/direct_bt/GATTHandler.cpp
@@ -245,7 +245,7 @@ void GATTHandler::l2capReaderThreadImpl() {
 }
 
 GATTHandler::GATTHandler(const std::shared_ptr<DBTDevice> &device, const int replyTimeoutMS)
-: device(device), deviceString(device->getAddressString()), rbuffer(number(Defaults::MAX_ATT_MTU)),
+: wbr_device(device), deviceString(device->getAddressString()), rbuffer(number(Defaults::MAX_ATT_MTU)),
   l2cap(device, L2CAP_PSM_UNDEF, L2CAP_CID_ATT), replyTimeoutMS(replyTimeoutMS),
   isConnected(false), hasIOError(false),
   attPDURing(number(Defaults::ATTPDU_RING_CAPACITY)),
@@ -256,6 +256,7 @@ GATTHandler::GATTHandler(const std::shared_ptr<DBTDevice> &device, const int rep
 GATTHandler::~GATTHandler() {
     eventListenerList.clear();
     disconnect(false /* disconnectDevice */, false /* ioErrorCause */);
+    services.clear();
 }
 
 bool GATTHandler::connect() {
@@ -334,6 +335,7 @@ bool GATTHandler::disconnect(const bool disconnectDevice, const bool ioErrorCaus
             }
         }
     }
+    std::shared_ptr<DBTDevice> device = getDevice();
 
     if( disconnectDevice && nullptr != device ) {
         // Cleanup device resources, proper connection state
@@ -470,6 +472,11 @@ bool GATTHandler::discoverPrimaryServices(std::vector<GATTServiceRef> & result)
     const std::lock_guard<std::recursive_mutex> lock(mtx_command); // RAII-style acquire and relinquish via destructor
     PERF_TS_T0();
 
+    std::shared_ptr<DBTDevice> device = getDevice();
+    if( nullptr == device ) {
+        ERR_PRINT("GATT discoverPrimary: Device destructed: %s", deviceString.c_str());
+        return false;
+    }
     bool done=false;
     uint16_t startHandle=0x0001;
     result.clear();
@@ -872,7 +879,8 @@ std::shared_ptr<GenericAccess> GATTHandler::getGenericAccess(std::vector<GATTCha
 
     for(size_t i=0; i<genericAccessCharDeclList.size(); i++) {
         const GATTCharacteristic & charDecl = *genericAccessCharDeclList.at(i);
-        if( _GENERIC_ACCESS != *charDecl.service->type ) {
+        std::shared_ptr<GATTService> service = charDecl.getService();
+        if( nullptr == service || _GENERIC_ACCESS != *(service->type) ) {
         	continue;
         }
         if( _DEVICE_NAME == *charDecl.value_type ) {
@@ -915,7 +923,8 @@ bool GATTHandler::ping() {
 
         for(size_t i=0; i<genericAccessCharDeclList.size(); i++) {
             const GATTCharacteristic & charDecl = *genericAccessCharDeclList.at(i);
-            if( _GENERIC_ACCESS != *charDecl.service->type ) {
+            std::shared_ptr<GATTService> service = charDecl.getService();
+            if( nullptr == service || _GENERIC_ACCESS != *(service->type) ) {
                 continue;
             }
             if( _APPEARANCE == *charDecl.value_type ) {
@@ -947,7 +956,8 @@ std::shared_ptr<DeviceInformation> GATTHandler::getDeviceInformation(std::vector
 
     for(size_t i=0; i<characteristicDeclList.size(); i++) {
         const GATTCharacteristic & charDecl = *characteristicDeclList.at(i);
-        if( _DEVICE_INFORMATION != *charDecl.service->type ) {
+        std::shared_ptr<GATTService> service = charDecl.getService();
+        if( nullptr == service || _DEVICE_INFORMATION != *(service->type) ) {
             continue;
         }
         found = true;