search

ARMmbed / wifi-ism43362

ISM43362 WiFi driver
17 stars 22 forks source link

Connection lost during socket_recv #38

Closed jeromecoutant closed 5 years ago

jeromecoutant commented 5 years ago

This fixes #29

In TCPSocket::recv() we need to return zero because closing a TCP socket it considered normal operation. Details in #29

@SeppoTakalo @screamerbg

jeromecoutant commented 5 years ago

Code updated

target platform_name test suite test case passed failed result elapsed_time (sec)
DISCO_L475VG_IOT01A-ARM DISCO_L475VG_IOT01A tests-netsocket-tcp TCPSOCKET_ENDPOINT_CLOSE 1 0 OK 1.31
SeppoTakalo commented 5 years ago

I don't agree with the first comment that @LMESTM gave. Now the driver is returning zero, only if background receiver thread notices that socket is closed.

In both cases the driver is calling _ism.check_recv_status() either directly from socket_recv() or from the background receiver thread socket_check_read()

So which one gets first, is just a timing.

I'm not sure if ISM43362 is capable of telling the difference of network failures, connection reset or orderly shutdown. If not, I recommend returning zero every time you notice that stream is closed and recv() is called.

Of course, if you can tell the difference, then it is OK to return error codes.

I did a quick look at the driver and seems to not do the same check when sending the data. If somebody tries to send data to closed socket, you can immediately return NSAPI_ERROR_CONNECTION_LOST

Other than those, this change looks OK. I'm just worrying about the timing.. it might be that you fixed it only 90% of the time.

LMESTM commented 5 years ago

@SeppoTakalo Ok I get your point - that's valid. the thing we can get a proper notification from the device when the connection was either lost or close from the remote side. So ok to move back to initial proposal from Jérôme.