-
Notifications
You must be signed in to change notification settings - Fork 431
Collected other pull request and Fixbug memory leaks and weakness argument pointer for tcp/ip callback function #173
base: master
Are you sure you want to change the base?
Conversation
Enable setting port
- Due to tcp_api_call_t msg declared local variant and used to as a pointer argument for the callback function. So this does not make sure that the memory holding the parameters of the pointer variant is handled right.
- AysncClient recv callback handle missing call pbuf_free()
This project seems quite dead since 4y... I would rather send PRs against this one now: https://github.com/esphome/AsyncTCP which is maintained by ESPHome (https://esphome.io) |
Thanks, |
This is not my repo ;-) If you leave it there, it will be lost. Nobody will act on it. But I have no relationship with any of these projects ;-) |
I got it, |
ee0155f
to
fd6447f
Compare
- Running async tcp by default core - Disable watchdog handler async tcp task - _tcp_recved() should be called when it has processed the data
@@ -417,7 +422,7 @@ static esp_err_t _tcp_write(tcp_pcb * pcb, int8_t closed_slot, const char* data, | |||
static err_t _tcp_recved_api(struct tcpip_api_call_data *api_call_msg){ | |||
tcp_api_call_t * msg = (tcp_api_call_t *)api_call_msg; | |||
msg->err = ERR_CONN; | |||
if(msg->closed_slot == -1 || !_closed_slots[msg->closed_slot]) { | |||
if(msg->closed_slot != -1 || !_closed_slots[msg->closed_slot]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this condition does not make any sense anymore: is it tested ? If msg->closed_slot == -1
, second expression is evaluated with an index being -1
. If not -1, second expression is not evaluated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TienHuyIoT : ^^ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
closed_slot
is int8_t so -1 means 255, but _closed_slots
is an array of 16.
I've grabbed most of the fixes in my fork: mathieucarbou/AsyncTCP#10 Still, please see my comment. A change in one condition looks really weird to me. |
while (connected() && !send()) { | ||
taskYIELD(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TienHuyIoT: when using SSE events in ESPAsycnWebServer, I think this busy loop can wait forever in case of a WIFi disconnection because the client is still seen there and connected
} | ||
pbuf_free(b); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pbuf_free change is OK but the remaining changes no.
See here for more details: tbnobody/OpenDTU#2326 (comment)
This causes a drastic slowdown of the readings.
Fixbug memory leaks
Fix crash tcp/ip api calls