Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Socket::sync if a client is disconnected #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lishuren
Copy link

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.

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.
@Lichtso
Copy link
Owner

Lichtso commented Aug 28, 2019

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?

@lishuren
Copy link
Author

lishuren commented Aug 29, 2019

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.

@Lichtso
Copy link
Owner

Lichtso commented Aug 29, 2019

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants