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

[Apollon77]

Hey,

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 addtional 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 "conenct" 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

[matthijskooijman]

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.

[Apollon77]

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;/

[agdl]

This issue was moved to arduino-libraries/Ethernet#7