Reuse of EthernetClient instances don't work when check connected() before stop()

[agdl]

From @Apollon77 on September 20, 2015 20:53

Copied from original issue: arduino/Arduino#3829

I use a library where the EthernetClient instance is created once and then reused.
This was not really working - the first call worked, then it did not connect for additional uses.

I tracked it down to the following:
The library did the following:

if (udata->client->theClient->connected())
{
  udata->client->theClient->stop();
}

And this was the reason! After all data was received the called server closed the connection. So the socket was in status CLOSE_WAIT and available returned 0 because all data was already read. That's why stop was never called.
And that's why the connect method found a still set _sock to a valid number and returned false.

I found the advice to use "if connected() then stop()" in several sources in the internet, but this brings big problems as seen here.

I think the fix should be to change the check in connected: All Sockets that are in CLOSE_WAIT are reused in any case ... so a call to connect should check also connected and call stop or simply reuse that socket also when _sock is a value < MAX_SOCK_NUM.

[agdl]

From @matthijskooijman on September 21, 2015 9:18

I wonder if the intent was to let a user call stop() unconditionally before reusing a socket, to prevent accidentally reusing a socket? I guess the check in connect() is to prevent keeping a socket open without any EthernetClient object pointing to it, losing the reference to it. However, if the socket is already closed, I guess it should be ok to reuse the socket even without calling stop() as proposed.

IIRC I ran into this issue a while back as well, and ended up just calling stop() before connect(), but that's probably not exactly as expected.

[agdl]

From @Apollon77 on September 21, 2015 9:28

The check in "connect" at the beginning is simply checking the current EthernetClient instance because it only checks that a valid Socket-number is assigned to the variable _sock (which states THE one socket used by EthernetCient at the moment). So it can't prevent "missing stop() calls on intact connections" cases ...

So my idea would be changing the

/if (_sock != MAX_SOCK_NUM) return 0;/

into something like

/if ( (_sock != MAX_SOCK_NUM) && connected(_sock) ) return 0;/