espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
12.98k stars 7.29k forks source link

pCharacteristic->notify() memory leak #8413

Open raphael-bmec-co opened 11 months ago

raphael-bmec-co commented 11 months ago

Board

Esp32devkit

Device Description

Plane module

Hardware Configuration

NA

Version

v2.0.9

IDE Name

CLion with platformIO

Operating System

Windows 11

Flash frequency

80

PSRAM enabled

yes

Upload speed

115200

Description

When calling notify @ ~10x/second with an MTU of 50-247, some backlog occurs, causing a memory leak and eventually the server becomes unresponsive to the client and logs esp_ble_gatts_send_ notify: rc=-1 Unknown ESP_ERR error.

The issue can be resolved by changed from 'notify' to 'indicate'. However, this is less performant and there is a clear leak if notifies fail to be received in full by a client.

Sketch

The BLE notify example sketch works fine. Just change the MTU - needs to be done client side to 247.

Debug Message

esp_ble_gatts_send_ notify: rc=-1 Unknown ESP_ERR error

Other Steps to Reproduce

Issues exists from 2.0.3 to 2.0.9. I have found from scattered mentions around the internet from years ago about a buffer getting full. I suspect the memory is still allocated as the read is still in progress when the next notify comes along and the memory is not freed correctly clogging up the buffer.

I have checked existing issues, online documentation and the Troubleshooting Guide

raphael-bmec-co commented 11 months ago

Seems related to, or the same as, https://github.com/nkolban/esp32-snippets/issues/773.

SuGlider commented 11 months ago

@raphael-bmec-co - Could you please post a basic BLE sketch example that replicates the issue (including an explanation about the necessary condition for the issue to happen)?

raphael-bmec-co commented 11 months ago

Mobile application steps

  1. Tested using lightBlue.
  2. Connect to the device.
  3. Select request MTU from the device settings menu:
  4. Set the MTU to 200 - the higher this value the faster the leak/crash.
  5. Select the characteristic from the list and tap subscribe.
  6. The ESP logs will show the free heap drop and then show eventually log the message [E][BLECharacteristic.cpp:537] notify(): << esp_ble_gatts_send_ notify: rc=-1 Unknown ESP_ERR error
  7. Note that the leak seems to arise from the client failing to read the notify in full. So the time taken for the leak and crash will depend on the environmental noise, number of BLE devices connected to the client and the priority (interval) assigned to the connection by the client.

Sample sketch

#include <BLEDevice.h>
#include <BLEServer.h>
#include <BLE2902.h>
#include <Arduino.h>

BLEServer* pServer = nullptr;
BLECharacteristic* pCharacteristic = nullptr;

#define SERVICE_UUID        "4fafc201-1fb5-459e-8fcc-c5c9c331914b"
#define CHARACTERISTIC_UUID "beb5483e-36e1-4688-b7f5-ea07361b26a8"

// Message to send.
static std::string message = "";

class MyServerCallbacks: public BLEServerCallbacks {

  void onDisconnect(BLEServer *pServer) override {
    pServer->getAdvertising()->start();
  }

  void onMtuChanged(
      BLEServer *pServer, esp_ble_gatts_cb_param_t *param
  ) override {
    message = std::string(param->mtu.mtu - 3,'*');
    log_i("MTU set to %u", param->mtu.mtu);
  }

};

void setup() {

  BLEDevice::init("esp_notify_issue");

  pServer = BLEDevice::createServer();
  pServer->setCallbacks(new MyServerCallbacks());

  BLEService *pService = pServer->createService(SERVICE_UUID);

  pCharacteristic = pService->createCharacteristic(
      CHARACTERISTIC_UUID,
      BLECharacteristic::PROPERTY_READ   |
          BLECharacteristic::PROPERTY_NOTIFY
  );

  pCharacteristic->addDescriptor(new BLE2902());

  pService->start();

  BLEAdvertising *pAdvertising = BLEDevice::getAdvertising();
  pAdvertising->addServiceUUID(SERVICE_UUID);
  BLEDevice::startAdvertising();
}

void loop() {

  pCharacteristic->setValue(message);
  pCharacteristic->notify();
  log_i("min free heap %lu", esp_get_minimum_free_heap_size());
  vTaskDelay(3/portTICK_PERIOD_MS);
}
SuGlider commented 11 months ago

Thanks @raphael-bmec-co - I'll try to solve it.

tichise commented 9 months ago

I too am having trouble finding a solution to this problem.

coofercat commented 1 month ago

I too can reproduce this. In my case, I have a wired mouse producing move events which I put into a RingBuffer in callbacks. I empty the ring buffer in the loop(), which will cause this error if I'm moving the mouse pretty fast. I can't say what speed the messages are being processed into notify(), nor am I tracking a memory leak, however, this seems to work:

while(mouse_events_buffer.pop(e)) {
  bleMouse.move(e.x, e.y, e.wheel);
  if(mouse_events_buffer.size() > 0) {
    delay(3);
  }
}

Some experimenting shows that delay(2) doesn't work, but 3 does. If I had to guess, I'd say that notify() doesn't block until the end of the BLE transmission, or is not re-entrant (but that really is just a guess).

Just to say, I tried changing my notify() to indicate(), which didn't work - so it's not an immediate drop in replacement for me.