Fix potential CORRUPT HEAP problem on libraries/BLE/src/BLEDevice.cpp

[CurryEx]

Description of Change

Let BLEDevice::removePeerDevice on libraries/BLE/src/BLEDevice.cpp serializable to avoid potential CORRUPT HEAP problem

Tests scenarios

excuse for my bad English
First of all, to recur this problem, you can try these code

#include <BLEDevice.h>
#include <Esp.h>

static BLEClient *pClient;
const BLEUUID serviceUUID("ebe0ccb0-7a0a-4b0c-8a1a-6ff2997da3a6");
const BLEUUID charUUID("ebe0ccc1-7a0a-4b0c-8a1a-6ff2997da3a6");
static BLERemoteService *pService;
static BLERemoteCharacteristic *pCharacteristic;

//a simple connect function, after connecting, to operate one servcice
bool connectAndSetNotify(BLEAddress address)
{
	if (pClient->connect(address) == true)
	{
		Serial.printf("signal strength %d \n", pClient->getRssi());

		pService = pClient->getService(serviceUUID);
		if (pService == nullptr)
		{
			Serial.print("connection broken: fail to get service");
			if (pClient->isConnected())
				pClient->disconnect();
			return false;
		}
		pCharacteristic = pService->getCharacteristic(charUUID);
		if (pCharacteristic == nullptr)
		{
			Serial.print("connection broken: fail to get characteristic");
			if (pClient->isConnected())
				pClient->disconnect();
			return false;
		}
		else
		{
			pCharacteristic->registerForNotify([](BLERemoteCharacteristic *pBLERemoteCharacteristic_THB, uint8_t *pData, size_t length, bool isNotify) {});
			return true;
		}
	}
	else
	{
		Serial.println("fail to connect");
		return false;
	}
}

void setup()
{
	Serial.begin(115200);
	BLEDevice::init("ESP32");
	pClient = BLEDevice::createClient();
}

void loop()
{
	BLEAddress addressAvailable("a4:c1:38:00:00:00");
	BLEAddress addressNotAvailable("a4:c1:38:00:00:01");
	//the first device MUST be connectable and the second device MUST be not connectable
	Serial.println("connecting to an available device");
	connectAndSetNotify(addressAvailable);
	pCharacteristic->registerForNotify(NULL);
	pClient->disconnect();

	//wait for disconnect
	delay(2000);
	Serial.println("connecting to a not available device");
	connectAndSetNotify(addressNotAvailable);
	Serial.println("!!can not reach!!");
	delay(10 * 1000);
}

In my opinion, there is no problem, but you will get CORRUPT HEAP exception just like issue #6961 and this

After debugging, I found there may have some situations that invoking this function simultaneously, such as while connecting to a not available device will invoke this function after rc != ESP_GATT_OK on BLEClient.cpp and ESP_GATTC_DISCONNECT_EVTon BLEClient::gattClientEventHandler.

Invoke simultaneously may cause concurrency problem of unsafe map m_connectedClientsMap

Therefore, my solution is to add a mutex lock to this function, if there is some better way to fix, please revise my PR

I have tested my Pull Request on framework-arduinoespressif32 3.20004.0(the environment I use is PIO I copy the version info from package.json) with ESP32 and ESP32-CAM Board

Related links

maybe related to issue #6961

[CLAassistant]

All committers have signed the CLA.

[me-no-dev]

shouldn't the same mux be used here and any place where m_connectedClientsMap is accessed?

[CurryEx]

As far as I'm noticed, the insert and other operations to m_connectedClientsMap will not be invoked concurrently. So, it should not be necessary🤔.

[me-no-dev]

are you positive that insert and find/erase can not happen at the same time?

[CurryEx]

erase operation will happen at the same time above the situation of this pull request. find operation will not cause concurrent problem. Regretfully, I haven't found enough evidence so far to promise if the insert operation will be invoked simultaneously in this library, however this is unlikely to happen. Therefore, I only made the BLEDevice::removePeerDevice serializable.