discard callback must be called on abort()#81
Conversation
| if (_discard_cb) { | ||
| // _pcb was closed here | ||
| _discard_cb(_discard_cb_arg, this); | ||
| } |
There was a problem hiding this comment.
@willmmiles: no sure if this block should be within the if (_pcb) { block or not. If yes, then maybe the same is true for close also and the latter should be updated too ?
What do you think ?
There was a problem hiding this comment.
#79 moved the _pcb check in AsyncClient::close() to be done just once in the LwIP context in _tcp_close_api, which then returns whether or not the dispose callback needs to be run after checking _pcb and the queue contents. The key reason is that _pcb could be nullptr while there's still a FIN or ERROR event pending, so even if it is, we need to purge the queue. I didn't want to replicate the purge code just to avoid the LwIP lock, though that would be a technically valid approach.
|
Ah, I'd noticed that ... but the trick was that Offhand, I'd probably recommend restoring the original semantics, just in case someone depends on |
|
Replaced by #82 |
This PR fixes a bug introduced in PR #79.
discarded callback must also be called when abort() is called.
Ref: ESP32Async/ESPAsyncWebServer#253