Skip to content
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

Memory leak due to aws_iot_shadow_init() #145

Closed
jaimeyu opened this issue Mar 7, 2018 · 8 comments
Closed

Memory leak due to aws_iot_shadow_init() #145

jaimeyu opened this issue Mar 7, 2018 · 8 comments
Labels

Comments

@jaimeyu
Copy link

jaimeyu commented Mar 7, 2018

Diagnosing a memory leak we're seeing in 2 of our apps with the heap tracing enabled. The app connects and disconnects from AWS periodically and we notice that we leak after each time it connects to AWS. We use a ESP32 and I filed a bug on their end because it is missing a ESP32 FreeRTOS free call but I realize that this (aws) repo is explicitly missing a aws_iot_thread_mutex_destroy() as well. I want to make sure this was on purpose, the porting guide says it aws_iot_thread_mutex_destroy() should be implemented, but it is never used... So ESP32 did implement this call but again, since it is never called, we just leak memory all the time.

These are the steps I took to root-cause our memory leak.
What I found was a very slow leak and xQueueGenericCreate() shows up in the heap tracer a lot.

11 allocations trace (300 entry buffer)

80 bytes (@ 0x3ffedfc8) allocated CPU 1 ccount 0x433e7330 caller 0x4008a342:0x4008a5a6:0x40123577:0x401225e8:0x40120d0c
0x4008a342: xQueueGenericCreate at /home/esp32/esp/esp-idf/components/freertos/./queue.c:2037

0x4008a5a6: xQueueCreateMutex at /home/esp32/esp/esp-idf/components/freertos/./queue.c:2037

0x40123577: aws_iot_thread_mutex_init at /home/esp32/esp/esp-idf/components/aws_iot/port/threads_freertos.c:37

0x401225e8: aws_iot_mqtt_init at /home/esp32/esp/esp-idf/components/aws_iot/aws-iot-device-sdk-embedded-C/src/aws_iot_mqtt_client.c:192

0x40120d0c: aws_iot_shadow_init at /home/esp32/esp/esp-idf/components/aws_iot/aws-iot-device-sdk-embedded-C/src/aws_iot_shadow.c:78

Ok, makes sense these mutexes are being created and dispose. So lets check if aws_iot_thread_mutex_destroy() is called to free the mutex with a grep.

$ pwd
/home/esp32/esp/esp-idf/components/aws_iot/aws-iot-device-sdk-embedded-C/src

$ grep -r aws_iot_thread_mutex_ *
aws_iot_mqtt_client.c: threadRc = aws_iot_thread_mutex_trylock(pMutex);
aws_iot_mqtt_client.c: threadRc = aws_iot_thread_mutex_lock(pMutex);
aws_iot_mqtt_client.c: return aws_iot_thread_mutex_unlock(pMutex);
aws_iot_mqtt_client.c: rc = aws_iot_thread_mutex_init(&(pClient->clientData.state_change_mutex));
aws_iot_mqtt_client.c: rc = aws_iot_thread_mutex_init(&(pClient->clientData.tls_read_mutex));
aws_iot_mqtt_client.c: rc = aws_iot_thread_mutex_init(&(pClient->clientData.tls_write_mutex));

So from my search, looks like the aws_iot port never frees the mutexes that it creates. This would explain the slow leak that we're seeing in our apps.

I did a test and moved aws_iot_shadow_init() so it is only init'd once at start up and this solved our memory leak issue. Unfortunately this produced a new bug. The bug is running out of client tokens due to this one line when calling aws_iot_shadow_register_delta(). The tokens are never zeroed until aws_iot_shadow_init() is called again.

I don't know what the solution is yet but I'm thinking of putting aws_iot_thread_mutex_destroy() into the disconnect logic and seeing if that fixes everything nicely (and properly). I'm wondering long term, to add an explicity aws_iot_mqtt_deinit() function to do all the clean ups...

@gordonwang0
Copy link
Contributor

Hi @jaimeyu,

Thanks for bringing this to our attention. We'll add this to our backlog of bugs to fix.

@jaimeyu
Copy link
Author

jaimeyu commented Mar 17, 2018

Hey, did the bug get fixed? Noticed it's closed without an associated commit.

@huguesBouvier
Copy link
Contributor

Hey,

Sorry my mistake, I was cleaning up.

@huguesBouvier huguesBouvier reopened this Mar 19, 2018
@escherstair
Copy link

Is there a roadmap with an expected date for the fix?

@gordonwang0
Copy link
Contributor

Hi @escherstair,

Unfortunately, we're unable to share our roadmap for this SDK publicly.

@escherstair
Copy link

Hi @gordonwang0 ,
I understand tha the roadmap for the new features cannor be shared publicly.
But this is an important bug; every memory leak is really dangerous for devices that must run 24/7.

For developers it's important to know if this SDK is an option or not for production devices (and not for prototypes only).

@gordonwang0
Copy link
Contributor

gordonwang0 commented Apr 7, 2018

Hi @escherstair,

Please take a look at #155, which includes (among other things) functions to free up resources used by the init functions.

Function signatures of aws_iot_mqtt_free and aws_iot_shadow_free.
Implementations of aws_iot_mqtt_free and aws_iot_shadow_free.

Feel free to leave code review comments.

@gordonwang0
Copy link
Contributor

Hi @jaimeyu,

Functions to free resources created by the init functions have been merged into the master branch. Closing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants