Skip to content

Fixes for memory leaks for Peers and around Services, Characteristics and Descriptors #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dreamstyler opened this issue Oct 18, 2020 · 1 comment
Labels
type: imperfection Perceived defect in any part of project

Comments

@dreamstyler
Copy link

dreamstyler commented Oct 18, 2020

I found some other issues after posting this #127

  1. Add ATTClass end function to clean up _peers and call from BLELocalDevice::end()
    can lead to memory leaks for _peers[i].device and other issues, especially if disconnect does not not happen nicely
void ATTClass::end()
{  
  for (int i = 0; i < ATT_MAX_PEERS; i++) {
    if (_peers[i].connectionHandle == 0xffff) {
      continue;
    }

    _peers[i].connectionHandle = 0xffff;
    _peers[i].role = 0x00;
    _peers[i].addressType = 0x00;
    memset(_peers[i].address, 0x00, sizeof(_peers[i].address));
    _peers[i].mtu = 23;

    if (_peers[i].device) {
      delete _peers[i].device;
      _peers[i].device = NULL;
    }
  }
}

void BLELocalDevice::end()
{
  GATT.end();
  ATT.end();
  HCI.end();

  // ....
 }
  1. Fix ref counting for BLELocalDescriptor in BLELocalCharacteristic
    Needed for 3 otherwise will delete BLELocalDescriptor prematurely.
BLELocalCharacteristic::BLELocalCharacteristic(const char* uuid, uint8_t properties, int valueSize, bool fixedLength) :
 BLELocalAttribute(uuid),
 _properties(properties),
 _valueSize(min(valueSize, 512)),
 _valueLength(0),
 _fixedLength(fixedLength),
 _handle(0x0000),
 _broadcast(false),
 _written(false),
 _cccdValue(0x0000)
{
 memset(_eventHandlers, 0x00, sizeof(_eventHandlers));

 if (properties & (BLENotify | BLEIndicate)) {
   BLELocalDescriptor* cccd = new BLELocalDescriptor("2902", (uint8_t*)&_cccdValue, sizeof(_cccdValue));
   
   cccd->retain(); // CALL RETAIN HERE
   _descriptors.add(cccd);
 }

 _value = (uint8_t*)malloc(valueSize);
}
  1. call retain twice GATTClass::addService(..)
    This is needed for 4, otherwise will cause a premature delete issue
void GATTClass::addService(BLELocalService* service)
{
  service->retain();
  _attributes.add(service);

  uint16_t startHandle = attributeCount();

  for (unsigned int i = 0; i < service->characteristicCount(); i++) {
    BLELocalCharacteristic* characteristic = service->characteristic(i);

    characteristic->retain();
    _attributes.add(characteristic);
    characteristic->setHandle(attributeCount());
    
    // add the characteristic again to make space of the characteristic value handle
    characteristic->retain();  // IMPORTANT: call this again since we are adding it twice
    _attributes.add(characteristic);

    ......
}
  1. perform clean-up in GATTClass::end()
    Need to preform proper housekeeping here, otherwise it will lead to memory leaks.
void GATTClass::end()
{
  clearAttributes(); // call this instead of _attributes.clear();
}
@dreamstyler dreamstyler changed the title Fixes for various memory leaks Fixes for memory leaks for Peers and around Services, Characteristics and Descriptors Oct 18, 2020
@dreamstyler
Copy link
Author

dreamstyler commented Oct 20, 2020

The following function should have a timeout to prevent getting locked up in some error condition.
Might be good to have a way for the user to detect if such errors happen so they can take action.

int HCIClass::sendAclPkt(uint16_t handle, uint8_t cid, uint8_t plen, void* data)
{
  const unsigned long start = millis();
  while (_pendingPkt >= _maxPkt) {
    Serial.print(';');
    const int a = poll();
    if (a < 0)
      Serial.print(a);

    if (millis() > (start + 3000))
    {
      Serial.println("HCIClass::sendAclPkt TIMEOUT");
      break;
    }
  }

Ideally, all while loops should have a timeout guard if they poll some peripheral.

@per1234 per1234 added the type: imperfection Perceived defect in any part of project label Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

2 participants