MediaTek-Labs / Arduino-Add-On-for-LinkIt-SDK

Arduino board support package for LinkIt 7697
https://docs.labs.mediatek.com/resource/linkit7697-arduino/en
34 stars 33 forks source link

Memory Leaks in BLE and WiFi uses cases #96

Closed pablosun closed 6 years ago

pablosun commented 6 years ago

https://zh.forum.labs.mediatek.com/t/7697-memory-leak/1382

2 memory leak cases are reported. This issue registers LBLE case. LWiFi case is registered as #100

Reproduce Steps

LBLE

Heap memory reduces when calling this repeatedly

LBLEPeripheral.advertiseAsBeacon(beaconData);
LBLEPeripheral.stopAdvertise();

repeatedly.

pablosun commented 6 years ago

I'll try to reproduce these 2 issues.

chengc843988 commented 6 years ago

The BLE API, when calling advertiseAsBeacon, API clear old data with .clear(), then copy the new data into new object , so it could not free release the old OBJECT in the heap. if have the API not to release old object, and uses the caller's object memory addreess, this issue is solved.

But, I don't think this a good solution!!

pablosun commented 6 years ago

@chengc843988 Thanks for the feedback, can you explain more on the clear() usage issue?

There are several data members that get clear()-ed:

  1. std::vector<LBLEAdvDataItem> m_advDataList;
  2. LBLEAdvDataItem
  3. std::vector<bt_gatts_service_rec_t*> m_records;

Item 1 and 2 seems pretty safe to be clear()-ed to me. We removes the element but keep the size of the vector, because we assume that the data size of advertisements are bounded by spec.

for item 3, it is true that the buffer pointed bybt_gatts_service_rec_t* never gets released. We assume that all services are declared statically and never gets released.

Perhaps you meant some other bugs? Can you clarify more?

Many thanks for your help.

chengc843988 commented 6 years ago
m_advDataList.clear(); private: std::vector m_advDataList; line: 115 LBLEAdvDataItem item; ==> create a new a object in heap. next call : run to line: 110 m_advDataList.clear(); // release all items !? ==> clear all item in the ? ==> item is a object in heap, if it release old item object ? (created by last run at line 115) I thins this line cause the memory leak issue!! I had made line: 115 to this *static* LBLEAdvDataItem item; and it works. and do not allocate any more with heap memory. I am not sure , Is there any side-effect if I made this change?
chengc843988 commented 6 years ago

LINE: 1066 (LBLEPeriphral.cpp) // remove the advertisement data // so that advertiseAgain() won't succeed // until advertise() API is called again. LBLEAdvertisementData* ptr=m_pAdvData.release(); delete ptr;