Restore abort dispose semantics #82
Conversation
Calling `AsyncClient::abort()` will now enqueue an error event, as it did prior to me-no-dev#79. This fixes memory leaks caused by missing dispose callbacks.
|
One interesting fact about this implementation is that it doesn't need to think about purging the queue in abort() -- if there was already a closing operation queued, |
There was a problem hiding this comment.
Pull Request Overview
This PR restores the original abort dispose semantics by ensuring that AsyncClient::abort() enqueues an error event instead of suppressing it. This fixes memory leaks that occurred when dispose callbacks were not properly triggered after PR #79.
Key changes:
- Simplifies the
_tcp_abort_apifunction by removing callback reset logic and event removal - Changes error handling to return
ERR_ABRTfor successful aborts instead ofERR_OK - Streamlines the
AsyncClient::abort()method to directly return the abort result
Comments suppressed due to low confidence (1)
src/AsyncTCP.cpp:922
- The closing brace for the
abort()function is misplaced. It should come after the return statement, not before the comment about _pcb being NULL.
}
| // _pcb is now NULL | ||
| } | ||
| return ERR_ABRT; | ||
| return _tcp_abort(&_pcb, this); |
There was a problem hiding this comment.
The return statement is missing proper formatting - it should be indented and the function should have a proper closing brace. The current code appears to have a formatting issue that could cause compilation errors.
|
Thanks a lot @willmmiles ! I will close the other pr as the correct fix was much more complex than I anticipated 😅🙂 |
|
@willmmiles : FYI I've sent you an invite to be part of the ESP32Async team. Feel free to accept or not as you wish: I know you are very busy... If you are on Discord, I could also add you to the maintainers channel. You could ping me with your username if you wish to do so. Thanks a lot for your help! |
Thank you! I'm happy to help out where I can. Thank you guys for bearing with all the new bugs. :) |
Calling
AsyncClient::abort()will now enqueue an error event, as it did prior to #79. This fixes memory leaks caused by missing dispose callbacks.This is an alternative to #81, which differs in that it calls the dispose callback inline in the
AsyncClient::abort()context instead. I suggest this approach as it is the original semantic of the AsyncTCP library, and involves less code.