[v5.1] memory being leaked in cJSON_Print() despite calling cJSON_free(jStr) (IDFGH-10250) #11512
Labels
Resolution: NA
Issue resolution is unavailable
Status: Done
Issue is done internally
Type: Bug
bugs in IDF
Comments
|
I am completely open to the idea of this being a false positive. But I've yet to be able to conclude that. It just grows and grows. I'll try and rule out bugs in my code, and bugs in HEAP_TRACE_STANDALONE more. ChatGPT is pretty helpful about this stacktrace.
|
github-actions
bot
changed the title
|
This is a lazy allocation in newlib C library. In this case, If you call sprintf again, no additional memory will be allocated. We have an internal non-thread-safe function |
|
Appreciate the response Ivan!
In my case, this is not the case. Every time I run my Bluetooth API call I leak another instance of this object. I am still investigating it. |
|
Ivan, are there any cases where |
|
No, I can't think of any such case. Calling sprintf in a loop results in a stable value of |
|
Here are all the objects. They're of various sizes (why...?), and slightly different stack traces. stack traces |
|
And the heap-trace-standalone summary count continues to go up, Note: the heap_trace_standalone tool is only ever enabled, never disabled, to make sure I don't miss any frees. |
|
Also, where is |
|
What about the actual heap size (rather than the heap trace records), does it increase? If yes, please try to create a code snippet which reproduces the issue and which you can share. The implementation of dtoa is here. |
|
Here are my findings: the cJSON "leak" seems fineIt takes a few calls but eventually _malloc_r: A different actual leak looks like a problem
Edit: I'm calling |
|
I was able to get a full backtrace using STACKMIRROR #11519. Given that stack trace, THIS IS A FALSE POSITIVE =) Linenoise has a history of 100 items. I would have needed to run my test 100 times before seeing the memory stabilize. There are still some remaining questions tho, in order or importance:
|
The backtrace is shown as corrupted because the backtracing code considers the address
As I understand, #10984 still contains some unresolved discussions/questions.
There are two parts to this question:
With regards to the last question, you might wonder, how do we debug the code which involves newlib? Typically, using the following steps:
I'm not sure. TBH after the last set of changes to heap-trace-standalone you probably know more about how it works than I do :) Someone will have to reproduce and debug this issue, I suppose.
This is debatable. Why pay the cost of allocating linenoise history up front, when much of the allocated space might not end up being used? If we end up running out of heap and fail to allocate a linenoise history entry, that's fine — linenoise tolerates that and will simply not write the item into the history. On the other hand, the space saved by not allocating all the history entries could be used to allocate something more important. |
|
Ivan, thank you for your incredible insightful and thoughtful response.
Great =) I'm glad this is an easy fix
In a debugging scenario, I was thinking just place all of newlib in IRAM, under some kconfig. But your steps for debugging newlib probably remove the need for this.
understood. If there is some way to enable "goto definition" and not precompile it into
This information is so useful! I will have to refer back to this next time I need to do this. A proper debug guide of this would be great. Perhaps even some tooling to simplify it.
Indeed. I did look for bugs there! but nothing obvious. My current belief is that this is not a bug in heap-trace-standalone. But it could be. I should try adding
Personal taste I suppose! It sure did make debugging this "leak" annoying tho haha. Slow growing heap in general. It would be useful if we could label certain callsites as "this is not a leak", but that might be impractical. |
espressif-bot
added
Status: In Progress
|
Going to close when the commit fixing SOC_IROM_MASK_HIGH gets pushed. |
|
Make sure
|
Any updates? Would be great to close this issue. |
|
closing this issue. Not sure if SOC_IROM_MASK_HIGH was triaged & fixed. But the original issue turned out to be a red-herring (caused by console command history) |
espressif-bot
added
Status: Done
espressif-bot
pushed a commit
that referenced
this issue
Sep 27, 2023
Fixes corrupted backtraces on S3 when a function is in ROM. Closes #11512
espressif-bot
pushed a commit
that referenced
this issue
Nov 21, 2023
Fixes corrupted backtraces on S3 when a function is in ROM. Closes #11512
movsb
pushed a commit
to movsb/esp-idf
that referenced
this issue
Dec 1, 2023
Fixes corrupted backtraces on S3 when a function is in ROM. Closes espressif#11512
espressif-bot
pushed a commit
that referenced
this issue
Dec 13, 2023
Fixes corrupted backtraces on S3 when a function is in ROM. Closes #11512
espressif-bot
pushed a commit
that referenced
this issue
Dec 20, 2023
Fixes corrupted backtraces on S3 when a function is in ROM. Closes #11512
hathach
pushed a commit
to adafruit/esp-idf
that referenced
this issue
Mar 27, 2024
Fixes corrupted backtraces on S3 when a function is in ROM. Closes espressif#11512
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
IDF version.
release/v5.1
Development Kit.
ESP32-S3 Dev Kit C
What is the expected behavior?
No memory should be leaked.
What is the actual behavior?
The more I run my api, the more I see this specific leak grow. I don't understand why. Why is sprintf allocating memory in the first place?
I've tripled checked that I am calling cJSON_free(jStr) ;
Steps to reproduce.
Ill need to get a better reproduction details.
The text was updated successfully, but these errors were encountered: