Double delete/free in BLERemoteService causes crash #3402
Description
I'm developing an arduino-esp32 program that connects to a BLE peripheral device. It might lose connection and reconnect to this device many times and I would like to eliminate or at least minimize any memory leaks. I have seen the free heap size shrink for each new reconnection until no more space is left. Until now I did not do any delete/free/cleanup in response to the BLE onDisconnect() event. I also haven't found any documentation about what to do in order to delete/free the created BLEClient, BLERemoteService|s, BLECharacteristic|s etc. I'm guessing that I should
delete pMyClient; /* instance of BLEClient */to cleanup, since the destructor of BLEClient will delete the BLERemoteService|s whose destructors call removeCharacteristics() to delete the BLECharacteristic|s, whose destructors in turn call removeDescriptors to delete the BLERemoteDescriptor|s.
The above "delete" causes a crash. This is due to a bug in removeCharacteristics() of BLERemoteService.cpp. Each BLECharacteristic gets deleted twice, one time per loop:
void BLERemoteService::removeCharacteristics() {
for (auto &myPair : m_characteristicMap) {
delete myPair.second;
//m_characteristicMap.erase(myPair.first); // Should be no need to delete as it will be deleted by the clear
}
m_characteristicMap.clear(); // Clear the map
for (auto &myPair : m_characteristicMapByHandle) {
delete myPair.second;
}
m_characteristicMapByHandle.clear(); // Clear the map
} // removeCharacteristicsIt is already fixed in the nkolban/esp32-snippets repository with this commit. Both maps m_characteristicMap and m_characteristicMapByHandle refer to the same BLECharacteristic|s. One of the for-loops must therefore be removed without replacement.
A local patch indeed works: No crash calling the statement above anymore. The memory leak is also gone or much smaller. I still ask myself if calling the statement is right thing to do. Normally a user should only call "delete" if "new" was called before (not counting "new" in libraries). Any hints are welcome.