Directly reset callbacks without going through lwip functions (fix #30)

[mathieucarbou]

fix #30

[willmmiles]

At least as I understood the documentation, LwIP frees the tcp_pcb when _tcp_error returns. Any dereferencing of _pcb in AsyncClient::_error, or any other event callback after that point, is a use-after-free and will result in corrupting memory. This was the major impetus behind the redesign in #21.

[mathieucarbou]

At least as I understood the documentation, LwIP frees the tcp_pcb when _tcp_error returns. Any dereferencing of _pcb in AsyncClient::_error, or any other event callback after that point, is a use-after-free and will result in corrupting memory. This was the major impetus behind the redesign in #21.

@me-no-dev suggested also another approach I will try, which is to immediately cleanup and after send the event in the queue (for the user callback only)

[willmmiles]

You can skip most of the API cleanup as the pcb is about to be freed; none of the other callbacks will get called again anyways. The key cleanup is to invalidate the close_slot so that any in-flight operation on the async thread won't accidentally try to dereference the now-released pcb.

[mathieucarbou]

@willmmiles @me-no-dev : just a heads up for this issue / PR:

We had 2 users having the issue (@hosseinaghaie and @moreArti - see #30).

@moreArti was able to test and confirm that the initial fix works. The initial fix was just about cleaning up states without calling the lwip functions.

void AsyncClient::_error(int8_t err) {
  if (_pcb) {
    TCP_MUTEX_LOCK();
    tcp_arg(_pcb, NULL);
    if (_pcb->state == LISTEN) {
      // tcp_sent(_pcb, NULL);
      _pcb->sent = NULL;
      // tcp_recv(_pcb, NULL);
      _pcb->recv = NULL;
      // tcp_err(_pcb, NULL);
      _pcb->errf = NULL;
      // tcp_poll(_pcb, NULL, 0);
      _pcb->poll = NULL;
      _pcb->pollinterval = 0;
    }
    TCP_MUTEX_UNLOCK();
    _free_closed_slot();
    _pcb = NULL;
  }

I have update this PR yesterday to update the fix with something we discussed with @me-no-dev : cleanup earlier within the lwip thread and only do the callbacks from the async_tcp task.

I am not sure about the _free_closed_slot(), if can be done earlier too, or if it has to be executed from the async_tcp task.

Do you know ?

Also, I have asked @moreArti to test again with this new fix.

Thanks everybody!