fix(mem): Fix memory leak in _accept() functions when _accept is called with a null pcb #25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mathieucarbou
changed the title
me-no-dev
reviewed
Feb 7, 2025
me-no-dev
approved these changes
Feb 7, 2025
vortigont
reviewed
Feb 8, 2025
…ed with a null pcb Fix inspired by PR #21 from @willmmiles
Member
Author
|
@vortigont : based on your suggestion, I have updated the PR to directly call tcp_abort. Tested again, works fine :-) I will let you approve and merge the PR, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix inspired by PR #21 from @willmmiles
AsyncTCP had a memory leak when receiving too many requests, even if the queue length is increased (256 in my testing). The problem was in the handling of the _accept function which was not handling all the possible use cases.
To reproduce, in main branch, trigger 3 times:
autocannon -c 32 -w 32 -a 96 -t 30 --renderStatusCodes http://192.168.4.1You should see a log similar than this one:
device-monitor-250207-213547.log
In my case, starting at
Uptime: 2 s, requests: 0, Free heap: 236932Ending at:
Uptime: 220 s, requests: 192, Free heap: 207624With this fix, you should see a log similar to this one:
device-monitor-250207-222548.log
[165298][E][AsyncTCP.cpp:1659] _accept(): _accept failed: pcb is NULLThis is the use case that was not handled before and was causing a leak 👍