This thread has been locked.
If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.
I have found a memory leak in the example code for the simplelink stack when running the example code (I developed my code from the SimpleBLECentral example). The leak occurs when, at the in-opportune time, the stack callback is executed just as the baseline code is attempting to dequeue a message. The code in question is in util.c in the routines Util_dequeueMsg and Util_enqueueMsg.
In this scenario, the code in the working loop attempts to dequeue a message from the applications message queue. If the stack interrupts at just the right time, it will place a new message on the queue which is pointed to by the message being dequeued in the baseline code. Then when the code returns from processing the stack interrupt and picks up where it left off in the baseline code, the previous message is finished being dequeued but the new message is not left in the queue but is still referenced by the dequeued message. Then when the code deletes the dequeued message and assicuated queue record objects it inadvertently leaves the new message left dangling in heap space. This eventually fragments the heap enough that no further messages can be allocated and the system simply stalls out due to an effectively exhausted heap space. Granted this takes a long time but it does make the requirement to reboot the system every couple of days or so in my system.
The fix is easy enough, in these two routines simply wrap the calls to Queue_enqueue and Queue_dequeue in critical section code. Here is my fix to the two util.c functions and it has corrected my memory leak issues on the cc2640 running the stack.
uint8_t Util_enqueueMsg(Queue_Handle msgQueue, Semaphore_Handle sem, uint8_t *pMsg) { queueRec_t *pRec; // Allocated space for queue node. if (pRec = (queueRec_t *)ICall_malloc(sizeof(queueRec_t))) { ICall_CSState csState = ICall_enterCriticalSection(); // <-- added code here pRec->pData = pMsg; Queue_enqueue(msgQueue, &pRec->_elem); ICall_leaveCriticalSection(csState); // <-- added code here // Wake up the application thread event handler. if (sem) { Semaphore_post(sem); } return TRUE; } // Free the message ICall_free(pMsg); return FALSE; } uint8_t *Util_dequeueMsg(Queue_Handle msgQueue) { ICall_CSState csState = ICall_enterCriticalSection(); // <-- added code here if (!Queue_empty(msgQueue)) { queueRec_t *pRec = Queue_dequeue(msgQueue); uint8_t *pData = pRec->pData; ICall_leaveCriticalSection(csState); // <-- added code here // Free the queue node // Note: this does not free space allocated by data within the node ICall_free(pRec); return pData; } ICall_leaveCriticalSection(csState); // <-- added code here return NULL; }
It could be argued that the code in the enqueue function doesn't need to be protected since it is only called in the callback function but nothing says you cannot call this from baseline code too.
Although this seems to have fixed the memory leak I have detected, I have to ask if these routines are used in the library code of the stack and if so, are they properly protected at those use sites?
Key thing to remember here is the Queue_enqueue and Queue_dequeue routines are not protected in a multi-threaded environment and therefore the user must implement the protection themselves.
Hi Jiminy,
After looking into this issue more, we have come up with a solution to ensure the enqueue/dequeue process is done in a thread-safe way.
For Util_enqueueMsg, we changed from using Queue_enqueue to Queue_put which is thread-safe. In Util_dequeueMsg, we disable HWIs before performing the dequeue operation. This change will be implemented in future TI BLE SDK releases.
To patch BLE SDK v2.2.0. replace the Util_enqueueMsg and Util_dequeueMsg in util.c (found in the src\common\cc26xx directory of your BLE SDK install) with the code below:
//Add to INCLUDES section #include <ti/sysbios/hal/Hwi.h> //Modify Util_enqueueMsg as follows: uint8_t Util_enqueueMsg(Queue_Handle msgQueue, Semaphore_Handle sem, uint8_t *pMsg) { queueRec_t *pRec; // Allocated space for queue node. if (pRec = ICall_malloc(sizeof(queueRec_t))) { pRec->pData = pMsg; // This is an atomic operation Queue_put(msgQueue, &pRec->_elem); // Wake up the application thread event handler. if (sem) { Semaphore_post(sem); } return TRUE; } // Free the message. ICall_free(pMsg); return FALSE; } //Modify Util_dequeueMsg as follows: uint8_t *Util_dequeueMsg(Queue_Handle msgQueue) { uint32_t key; key = Hwi_disable(); // Queue_empty is not an atomic operation, it needs to be protected with CS. if (!Queue_empty(msgQueue)) { queueRec_t *pRec = Queue_dequeue(msgQueue); uint8_t *pData = pRec->pData; Hwi_restore(key); // Free the queue node // Note: this does not free space allocated by data within the node. ICall_free(pRec); return pData; } Hwi_restore(key); return NULL; }
I always use Queue_put and Queue_get. But now just learned that Queue_empty() is not thread-safe.
What about Queue_head()? Is that not thread-safe as well?
Hi Rachel,
If Queue_head() is thread-safe, would Queue_empty() be thread-safe as well (therefore Hwi disable/enable protection is not required) because it merely checks the queue is empty or not? Please double check for us because it will save a lot of code change and regression.
../ming