Sign in to follow this  
Diventurer

[Solved] WinSock TCP - Crash if buffer over around 5000

Recommended Posts

Hi, so I am currently making a game with WinSocket, and I want the server to send maps to the client...
However, I've got a problem...

I have a #define which is the default buffer length, if I change that to over 4997, it will make the client freeze and not respond until the server gets closed.
Both recv and send uses that buffer length.

Is there a way to make it possible to send and recv more than 4997 bytes?

I'd guess it's not the default with everyone, but it seems to be for me... Any help is greatly appreciated :)

Share this post


Link to post
Share on other sites
Hmm, I'm really unsure how I would do that, as I can't send more than 4977 bytes per send...
Making a different code for sending and receiving each map is not too efficient...

I have no idea how the code can cause it to be honest, everything should depend on the #define ...

Thanks for your reply though

Edit:
Weird, I typed in 5499 or something and it worked, now I changed to 7200 to check if it actually works, but it crashes again...

Share this post


Link to post
Share on other sites
I will do something like that if there is no other choice, but for now I want to try to see if I can get rid of this limit... As it shouldn't be limited.

Edit:
It's weird how it seems the max is increasing... I can now send and recv around 5720...

Any ideas?

Edit 2:
Ehm, I'm using TCP by the way... I don't think I should manually split the packets ...

Share this post


Link to post
Share on other sites
Ooh, I'm so glad that you didn't mention any limit :)

Here are the send and recv usage, I think this should be enough?
If not, then please ask.

[code]#define BUFFERSIZE 9000

bool SendData(SOCKET _sdsock, string _sddata, bool _sdlog)
{
const char *_sdtemp = Char(_sddata);
if (send(_sdsock, _sdtemp, BUFFERSIZE, 0) == SOCKET_ERROR) {
if (_sdlog) {
cout << "\nError while trying to send data: " << _sdtemp << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
return false;
} else {
if (_sdlog) {
cout << "\nSuccessfully sent data: " << _sdtemp << endl;
}
return true;
}
}

bool RecvData(SOCKET _rdsock, string &_rddata, bool _rdlog)
{
char _rdtemp[BUFFERSIZE + 1];
if (recv(_rdsock, _rdtemp, BUFFERSIZE, 0) <= 0) {
if (_rdlog) {
cout << "\nError while trying to receive data: " << _rdtemp << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
_rdtemp[0] = 0;
return false;
} else {
if (_rdlog) {
cout << "\nSuccessfully received data: " << _rdtemp << endl;
}
_rddata = _rdtemp;
_rdtemp[0] = 0;
return true;
}
}[/code]

Share this post


Link to post
Share on other sites
You're outputting a possibly non-terminated string; recv doesn't add any 0s to the data. If the data is split up, you'll get a fragment of the original string without a trailing 0. std::string has a constructor where you can pass a const char * and the length:
[code]
ssize_t rxSize = recv(_rdsock, _rdtemp, BUFFERSIZE, 0)
...
_rddata = std::string(_rdtemp, rxSize);

// or you can use the assign function:
_rddata.assign(_rdtemp, rxSize);
[/code]

Why are you doing _rdtemp[0] = 0 before returning? It goes out of scope anyway.


[font="arial, verdana, tahoma, sans-serif"][size="2"][quote name='Segun24' timestamp='1318160231' post='4870755']
I will do something like that if there is no other choice, but for now I want to try to see if I can get rid of this limit... As it shouldn't be limited.

Edit:
It's weird how it seems the max is increasing... I can now send and recv around 5720...

Any ideas?

Edit 2:
Ehm, I'm using TCP by the way... I don't think I should manually split the packets ...
[/quote]

[/size][/font][font="arial, verdana, tahoma, sans-serif"][size="2"]The data can be split up, and multiple sends can be concatenated together. If you send 100 bytes, you're not guaranteed to get 100 bytes from one call to recv. BTW; there are no "packets" in this layer. :)[/size][/font] Edited by patrrr

Share this post


Link to post
Share on other sites
If the 'Char' function (is it a function or a type...?) gives less data than BUFFERSIZE, then that's a problem of course; send would read too much from memory. I strongly suggest you to consider all the input in this thread, and learn why the input is sensible. This is a sensitive part of the code, and strange bugs can/will occur in the future if you don't get it right.

Share this post


Link to post
Share on other sites
Your networking code contains a number of errors. [url="http://msdn.microsoft.com/en-us/library/windows/desktop/ms740149(v=vs.85).aspx"]send() is documented[/url] as potentially "partially sending" the specified buffer. You need to call send() in a loop to ensure all the bytes are sent.

Something like this: (warning: code neither compiled nor tested)
[code]
bool SendData(SOCKET socket, string data, bool log)
{
const char *buffer = data.c_str(); // What is Char() ?
int sent = 0;
while(sent < data.length())
{
int bytes = send(socket, buffer, data.length() - sent, 0);
if(bytes == SOCKET_ERROR)
{
if(log)
{
cout << "\nError while trying to send data: " << data.c_str() << endl;
cout << '\n' << sent << " bytes sent" << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
return false;
}
else
{
sent += bytes;
buffer += bytes;
}
}

if(log)
{
cout << "\nSuccessfully sent data: " << data.c_str() << endl;
}
return true;
}
[/code]

Likewise, your recv() code is perfectly willing to accept partial messages and treat them as whole ones, or to merge multiple messages and treat them as a single one.

To solve this, you need to come up with a protocol that allows the sender to specify which data makes up which messages. Two common solutions are to prefix each message with the number of bytes in the message, or to use a delimiter value to break up the messages. Binary protocols usually use the former, while text protocols often use the latter. Another solution is to use packet type identifiers - each identifier maps to a particular packet type, with a known length.

You'll need a persistent application buffer per-client to do this. This buffer needs to be passed into successive calls with the same socket. A good idea is to package the buffer and socket into a structure to keep them together.

It might look something like this:
[code]
struct Client
{
SOCKET socket;
uint32_t nextPacketSize;
std::vector<char> buffer;

Client(SOCKET socket)
:
socket(socket),
nextPacketSize(0),
buffer(BUFFER_SIZE)
{
}

bool RecvData(string &message, bool log);
};

bool Client::RecvData(string &message, bool log)
{
int bytes = recv(socket, buffer, buffer.size(), 0);
if (bytes <= 0)
{
if (log)
{
cout << "\nError while trying to receive data" << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
return false;
}

if (log)
{
cout << "\nSuccessfully received " << bytes << " bytes." << endl;
}

// We haven't determined how big the next packet is yet
if(nextPacketSize == 0)
{
// Check if there is a full length prefix in the buffer...
if(buffer.size() > sizeof(nextPacketSize))
{
// Function removes sizeof(uint32_t) from buffer, in a known endian format
nextPacketSize = extractUint32(buffer);

// Sanity check the client packet length
if(nextPacketSize == 0 || nextPacketSize > BUFFER_SIZE)
{
// Handle protocol error...
return false;
}
}
else
{
// Haven't got a full message length prefix yet.
return false;
}
}

// Does our buffer contains a full message?
if(nextPacketSize <= buffer.size())
{
// Copy the data
message.assign(buffer.begin(), buffer.begin() + nextPacketSize);
// Clear that part of the buffer
buffer.erase(buffer.begin(), buffer.begin() + nextPacketSize);
// We're waiting for the packet header again
nextPacketSize = 0;
// Signal success
return true;
}
// Still waiting for a full message...
return false;
}
[/code]
Again, code neither compiled nor tested.

Share this post


Link to post
Share on other sites
Thanks, [b]rip-off[/b], I'll fix that stuff up : )

The[color="#800080"] Char[/color][color="#808000"]([/color][color="#0000FF"]string[/color][color="#808000"])[/color] function returned a [color="#0000FF"]char[/color][color="#808000"]*[/color] of the [color="#0000FF"]string[/color] given, I just like to make functions for stuff to make it cleaner...cleaner for me anyhow : )

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this