netLink icon indicating copy to clipboard operation
netLink copied to clipboard

Fix Socket::sync if a client is disconnected

Open lishuren opened this issue 6 years ago • 3 comments

If a client is disconnected during the transfer progress, socket status will be BUSY. Socket::sync will not be able to write any data and should return EOF instead.

lishuren avatar Aug 28 '19 07:08 lishuren

You mean the connection is not shut down gracefully, instead the ACK timers run out and the output buffer fills up. Interesting, is there a specific example where it would help to return EOF instead of 0?

Lichtso avatar Aug 28 '19 14:08 Lichtso

Hi Lichtso,

First of all, thanks for your work and your project. netLink is such a neat and elegant lib. I like it very much.

You are correct, the connection is not shut down gracefully. I am using netLink in the communication between a windows PC and an embedded linux. PC Server receives a request from a client, and then processes the request, finally echoes back. All the actions are done in onReceiveMsgPack. In my case, the embedded system might be shut down accidentally, while PC box is still sending large data set. At the moment, the PC box's socket status is BUSY.

Here is an example: Adding the following code as server side, send an message from a client and immediately kill the client. The server takes time to send all bytes to socket.

socketManager.onReceiveMsgPack = [](netLink::SocketManager* manager, std::shared_ptr<netLink::Socket> socket, std::unique_ptr<MsgPack::Element> element) {
	char* dst = new char[10000000];
	netLink::MsgPackSocket& msgPackSocket = *static_cast<netLink::MsgPackSocket*>(socket.get());
	msgPackSocket << MsgPack__Factory(MapHeader(1));
	msgPackSocket << MsgPack::Factory("Data");
	msgPackSocket << std::unique_ptr<MsgPack::Element>(new MsgPack::Binary(10000000, dst)); };

I have tested to return EOF or 0. Both of them work. Based on streambuf.sync document, it says "Returns zero, which indicates success. A value of -1 would indicate failure." I am assuming it is failed scenario and use EOF. If using 0 instead of EOF, sync() will keep re-run until time out, which takes more time to disconnect the client than EOF.

lishuren avatar Aug 29 '19 07:08 lishuren

I will have to investigate this further:

  1. Actually, if Socket::send fails it raises an exception which is caught by Socket::sync() and results in an EOF. And further sync() calls would do a send retry, but that is ok because:
  2. What if the remote did not shut down but there is a network congestion, thus the connection could recover?
  3. Without the socket manager there is currently no way to get back from BUSY to READY state, so maybe:
  4. It is not used anywhere in netLink so; Should we drop the BUSY state all together?

Lichtso avatar Aug 29 '19 10:08 Lichtso