me-no-dev / ESPAsyncTCP

Async TCP Library for ESP8266
GNU Lesser General Public License v3.0
755 stars 392 forks source link

onAck hander not firing #160

Closed bertmelis closed 3 years ago

bertmelis commented 3 years ago

I've been stress testing my TCP queue and I ran into something that I don't quite understand.

The onAck callback is only called when unacked is zere: https://github.com/me-no-dev/ESPAsyncTCP/blob/15476867dcbab906c0f1d47a7f63cdde223abeab/src/ESPAsyncTCP.cpp#L552

On sending, the _tx_unacked_len counter in incremented with the amount sent. On acking, the counter is decremented. And if zero, onAck is fired. Could it be that, should there be a disconnect between the two (also think of outside TCP like when I just pull my switches power cord), _tx_unacked_len stays like it is. And when a new connection is setup, the counter increments and decrements but never reaches zero. And so the onAck never fires again.

--> sending data
000017.036 [ASYNC_TCP] _sent[1]: 1460, unacked=1460, acked=1460, space=1460
000017.038 [ASYNC_TCP] _sent[1]: 1460, unacked=   0, acked=2920, space=2920
--> ack called, I can remove data from queue safely
--> sending more data
000018.654 [ASYNC_TCP] _recv[1]: pb == NULL! Closing... 0
--> disconnected
--> connected again
--> sending more data
000028.667 [ASYNC_TCP] _sent[1]: 1460, unacked=4380, acked=1460, space=1460
000028.669 [ASYNC_TCP] _sent[1]: 1460, unacked=2920, acked=2920, space=2920
--> from here `unacked` will never be zero again and onAck is never called.

Is there any reason why _tx_unacked_len is not reset to zero on (re)connect?

bertmelis commented 3 years ago

Is there any reason why _tx_unacked_len is not reset to zero on (re)connect?

For the record: I did try with resetting _tx_unacked_len to zero in void AsyncClient::_connected and I could not make my application to fail anymore. So this solves my issue. However, I'm not a TCP expert by far and I don't know the architecture of this library so I'm not 100% sure of any side-effects.

bertmelis commented 3 years ago

When other people also suffer from this issue: a workaround is to create the asyncclient dynamically and delete on disconnect.

stale[bot] commented 3 years ago

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

bertmelis commented 3 years ago

not stale.

stale[bot] commented 3 years ago

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

stale[bot] commented 3 years ago

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.