sf::TcpSocket, sf::Packet and NonBlocking behavior...

[Daid]

I'm trying to use SFML's TCP networking implementation to do client/server code for a game I'm working on.

However, I ran into a problem. I made all code single threaded, so I put all sockets into NonBlocking mode with setBlocking(false); And then I send sf::Packets trough it.
However, it looks like I'm getting corrupted packets at the receiving end when I try to send a lot of data at once (usually at the initialization of the client state, which needs a lot of data)


I think I tracked it down to the sf::TcpSocket::send function:
https://github.com/SFML/SFML/blob/master/src/SFML/Network/TcpSocket.cpp#L220

The problem here, is that when you are in NonBlocking mode, you can have a partial send on the ::send() function. As the buffer at the OS level is full. On the next retry of ::send it will return -1 with an EWOULDBLOCK error. And thus exiting the send function. Reporting to the caller a status of "NotReady", however, it did partially send the message. Thus corrupting the packet on the receiving side. (And pretty much corrupting the whole stream)


I do not want the send function to block (else I could put it into blocking mode before sending) as this could block the whole server if 1 client went missing (which fills up the OS buffer, a block before the socket is disconnected).
Sending with threads could be done, but as SFML lacks a Queue or Conditionals then this could become very tricky very fast, not even sure it can be done without polling, which I rather avoid.

And as TcpSocket::send also fails to report how much data it actually send, else I could keep track of how much I still needed to send myself.


Not sure where to go from here...

[binary1248]

Yeah... this is a known problem, and short of an API change, there isn't a satisfactory way to fix it. We can't make such a noticeable API change before SFML 3, so until then there is no simple workaround for this.

One possibility might be to split your huge data up into smaller segments and send explicit acknowledgements back to the sender after the receiver has received a whole segment. This way you can participate a bit in the rate control and try to make sure the send buffer doesn't become full too fast. They typically aren't too small either, something on the order of 64KB, so you would have to send a relatively huge chunk of contiguous data in one burst to cause it to become full.

[Daid]

Yeah... this is a known problem, and short of an API change, there isn't a satisfactory way to fix it. We can't make such a noticeable API change before SFML 3, so until then there is no simple workaround for this.

May I disagree there?

Option A: The sf::TcpSocket::send could store the rest of the sf::Packet if sending partially failed. Reporting success back to the application, and when the next call to any socket function is made it's tried again to send it. No other data is send till this data has been send correctly.
Option B: Add a sf::TcpSocket::send(void *data, std::size_t size, std::size_t &actual_send_size) function to the socket API. Adding functions doesn't break backwards compatibility.
Option C: Add a sf::Conditional so proper threading can be implemented. Still an addition, so no backwards compatibility breaking. Not to mention that I've seen more request for this basic thread synchronization feature.

Yeah... this is a known problem

Documentation! Documentation! Documentation! Would have saved me TONS and TONS of time if the API documentation mentioned this tiny fact. Pretty much spend the whole day yesterday trying to track this down, figuring it was a bug in my own code at first.
Not to mention the few play testing sessions with 6~10 people this bug ruined so far.

One possibility might be to split your huge data up into smaller segments and send explicit acknowledgements back to the sender after the receiver has received a whole segment. This way you can participate a bit in the rate control and try to make sure the send buffer doesn't become full too fast. They typically aren't too small either, something on the order of 64KB, so you would have to send a relatively huge chunk of contiguous data in one burst to cause it to become full.

May I point out that that is a REALLY shitty solution? That implementing part of TCP on top of TCP. Also has the nice potential for race conditions (what if buffers on both sides are full, then you cannot send the acknowledge). Waste of bandwidth, and horrible if your round-trip times are a bit longer.



Think I just figured a workaround. I can overload the sf::TcpSocket class, then I can access the sf::Socket::getHandle() function, which allows me to call the OS ::send function directly, catching the partial sends.


(Sorry that I'm not replying in a nice manner. I can be like that...  )

[binary1248]

Option A: The sf::TcpSocket::send could store the rest of the sf::Packet if sending partially failed. Reporting success back to the application, and when the next call to any socket function is made it's tried again to send it. No other data is send till this data has been send correctly.

This is would still not fix the issue with sf::TcpSocket::send(const void* data, std::size_t size) which is the real root of the problem. The packet overload depends on this one, so fixing this one should be considered before anything else.

Option B: Add a sf::TcpSocket::send(void *data, std::size_t size, std::size_t &actual_send_size) function to the socket API. Adding functions doesn't break backwards compatibility.

While SFML doesn't normally add overloads just to work around bugs, this could work. However, considering work on SFML 3 should start in the not too distant future, one has to ask themselves how clean of a solution this is for the remainder of the lifetime of SFML 2 before going ahead and implementing this.

Option C: Add a sf::Conditional so proper threading can be implemented. Still an addition, so no backwards compatibility breaking. Not to mention that I've seen more request for this basic thread synchronization feature.

Can you describe this "proper threading"? I know SFML doesn't support the more... powerful threading primitives, but I don't see the current API as being such a limiting factor when it comes to writing threaded applications. Sure it could be better, but saying that a "basic thread synchronization feature" is missing implies that it is more flawed than it actually is. Also, do you have links to these requests? Because I don't remember seeing that many of them on the forum.

Documentation! Documentation! Documentation! Would have saved me TONS and TONS of time if the API documentation mentioned this tiny fact. Pretty much spend the whole day yesterday trying to track this down, figuring it was a bug in my own code at first.
Not to mention the few play testing sessions with 6~10 people this bug ruined so far.

I am very sorry for your loss of time, but you do realize that the documentation that you see online is more than 1.5 years old right? And when I say this is a known bug, I should have probably added that it has been known for less than 1.5 years of time. So if we had added a note, unless you grab and use the latest master with the corresponding documentation, you wouldn't have seen it anyway. I know this sounds like an excuse, and it probably is, but that is unfortunately the way things are. We are working on reducing the time between releases, so in the future, hopefully stuff like this will be made easier for people to discover without too much effort.

May I point out that that is a REALLY shitty solution? That implementing part of TCP on top of TCP. Also has the nice potential for race conditions (what if buffers on both sides are full, then you cannot send the acknowledge). Waste of bandwidth, and horrible if your round-trip times are a bit longer.

there isn't a satisfactory way to fix it

It was a suggestion, and probably the least painful of all possible workarounds, so you can take it or leave it. I took 2 minutes to type that suggestion, and you probably took the same amount of time to type that answer. I don't require an acknowledgement to everything I suggest, so maybe it is better we didn't waste this much time in the future.

Think I just figured a workaround. I can overload the sf::TcpSocket class, then I can access the sf::Socket::getHandle() function, which allows me to call the OS ::send function directly, catching the partial sends.

That could work yes.

(Sorry that I'm not replying in a nice manner. I can be like that...  )

Oh, it can get much worse around here, and even more time is wasted in those discussions .

[Laurent]

Option C: Add a sf::Conditional so proper threading can be implemented. Still an addition, so no backwards compatibility breaking. Not to mention that I've seen more request for this basic thread synchronization feature.

The standard library of C++11 has everything that you need for threading.