Jump to content

  • Log In with Google      Sign In   
  • Create Account


Receive issues with recv()


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
6 replies to this topic

#1 Chicktopus   Members   -  Reputation: 100

Like
0Likes
Like

Posted 26 March 2011 - 02:24 PM

G'day to you!

I'll get straight to the point: I'm having some trouble with sending data from my server to my client and vice versa. Basically, data is continuously sent between the two, and this works fine for the most part. Problem is that at some point data will not be received by one or the other (possibly because of packet loss? I dunno). It will always stop receiving at the same point. The point at which it stops seems to be directly affected by the number of instructions between the calls to send() and recv(). i.e. if I had a piece of code that said:

	m_iResult = ReceiveData(m_clientSocket, m_temp);

	if (m_temp !=  m_controlArr.at(i).y)
	{
		printf("Send mismatch %d\n");
	}


The recv() would not receive after, say the 20th time?

Now, if I removed the validation test to just:


	m_iResult = ReceiveData(m_clientSocket, m_temp);

The recv() might fail after only the 8th piece of data had been sent.

Here's the important parts of the code for your reviewing pleasure:
SERVER

	// Receive until the peer shuts down the connection
    do {

			m_iResult = ReceiveData(m_clientSocket, m_radius);
			m_iSendResult = WriteData(m_radius, m_clientSocket);
			m_iResult = ReceiveData(m_clientSocket, m_mousePos.x);
			m_iSendResult = WriteData(m_mousePos.x, m_clientSocket);
			m_iResult = ReceiveData(m_clientSocket, m_mousePos.y);
			m_iSendResult = WriteData(m_mousePos.y, m_clientSocket);
			m_iResult = ReceiveData(m_clientSocket, m_mousePos.z);
			m_iSendResult = WriteData(m_mousePos.z, m_clientSocket);

			vector<Vertex> m_controlArr;
			m_ownedCount = 0;
			for (int i = 0; i < m_grid; i++)
			{
				for (int j = 0; j < m_grid; j++)
				{
					m_pos = g_sphereArr[i][j].GetPosition();
					if ((m_pos.x < m_radius) && (m_pos.y < m_radius))
					{
						if (g_sphereArr[i][j].GetOwnerShip() != 4)
						{ 
							g_sphereArr[i][j].SetOwnership(0);
						}
						m_player[0].SetOwnership(i, j); 
						m_controlArr.push_back(m_pos);
						m_ownedCount++;
					}
				}		
			}
		
			m_iSendResult = WriteData(m_ownedCount, m_clientSocket);
			vector<float> m_sendBuff;
			float m_temp;
			for (int i = 0; i < m_ownedCount; i++)
			{
				printf("%d\n", i);
				m_iSendResult = WriteData(m_controlArr.at(i).x, m_clientSocket);
				m_iResult = ReceiveData(m_clientSocket, m_temp);
				if (m_temp !=  m_controlArr.at(i).x)
				{
					printf("Send mismatch %d\n");
				}
				m_iSendResult = WriteData(m_controlArr.at(i).y, m_clientSocket);
				m_iResult = ReceiveData(m_clientSocket, m_temp);
				if (m_temp !=  m_controlArr.at(i).y)
				{
					printf("Send mismatch %d\n");
				}
				m_iSendResult = WriteData(m_controlArr.at(i).z, m_clientSocket);
				m_iResult = ReceiveData(m_clientSocket, m_temp);
				if (m_temp !=  m_controlArr.at(i).z)
				{
					printf("Send mismatch %d\n");
				}
			}
		    printf("Bytes sent: %d\n");
	
    } while (m_iSendResult > 0);

CLIENT

	int m_result = 512;
	g_socket.WriteData(m_radius);			//Send radius
	m_result = g_socket.ReadData(m_radius);
	g_socket.WriteData(g_mousePos.x);				//Send position data
	m_result = g_socket.ReadData(g_mousePos.x);
	g_socket.WriteData(g_mousePos.y);
	m_result = g_socket.ReadData(g_mousePos.y);
	g_socket.WriteData(g_mousePos.z);
	m_result = g_socket.ReadData(g_mousePos.z);

	GLfloat m_sphereSize = 3.0f;
	Vertex m_spherePos;
	int m_receiving = 0;

	g_socket.ReadData(m_controlCount);
	Sphere* m_sphereArr = new Sphere[m_controlCount];
	vector<Vertex> m_sphere;
	for (int i = 0; i < m_controlCount; i++)
	{
		m_sphereArr[i].Create(m_sphereSize, 32, 32);
		printf("%d\n", i);
		m_result = g_socket.ReadData(m_spherePos.x);
		g_socket.WriteData(m_spherePos.x);
		m_result = g_socket.ReadData(m_spherePos.y);
		g_socket.WriteData(m_spherePos.y);
		m_result = g_socket.ReadData(m_spherePos.z);
		g_socket.WriteData(m_spherePos.z);
		m_sphere.push_back(m_spherePos);
	}

...and here's the send function I'm using in both...

int Socket::WriteData(float p_data)
{
	std::string m_tmp = FloatToString(p_data).c_str();
	char* m_strToChr = (char*)m_tmp.c_str();
	g_sendBuf = m_strToChr;
	int m_sndResult = send(g_connectSocket, g_sendBuf, (int)strlen(g_sendBuf), 0);
	std::cout << "Sent: " << g_sendBuf << std::endl;
	if (m_sndResult == SOCKET_ERROR)
	{
		printf("Send failed: %d\n", WSAGetLastError());
        	closesocket(g_connectSocket);
        	WSACleanup();
		return 1;
	}
	return m_sndResult;
}

...and the receive function:

int Socket::ReadData(float &p_data)
{
	char m_recvbuf[DEFAULT_BUFLEN];
	int m_recvBufLen = DEFAULT_BUFLEN;
	std::string m_buf;
	int m_iResult = -1;
	m_iResult = recv(g_connectSocket, m_recvbuf, m_recvBufLen, 0);
	
	if (m_iResult == INVALID_SOCKET)
	{
		printf("Receive failed: %d\n", WSAGetLastError());
        closesocket(g_connectSocket);
        WSACleanup();
		return 1;
	}
	for (int i = 0; i < m_iResult; i++)
	{
		m_buf.push_back(m_recvbuf[i]);
	}
	std::istringstream m_strToFloat(m_buf);
	m_strToFloat >> p_data;
	std::cout << "Recieved: " << p_data << std::endl;
	return m_iResult;
}


Any help would be greatly appreciated, I've been stuck on this for a while now.

Note that this is using blocking as the data must be sent in sync. Later I need to add another three clients, and yes, I know this code ain't pretty, but at this point as long as it works I'll be happy.

Sponsor:

#2 hplus0603   Moderators   -  Reputation: 4970

Like
0Likes
Like

Posted 26 March 2011 - 06:48 PM

I'll get straight to the point: I'm having some trouble with sending data from my server to my client and vice versa. Basically, data is continuously sent between the two, and this works fine for the most part. Problem is that at some point data will not be received by one or the other (possibly because of packet loss? I dunno). It will always stop receiving at the same point. The point at which it stops seems to be directly affected by the number of instructions between the calls to send() and recv(). i.e. if I had a piece of code that said:


send() *may* block if the output buffer is full.
recv() *will* block until it can receive at least one byte.
If you get to the point where both programs are waiting for data at the same time, you will deadlock.
Also, if one end is falling behind, and send() blocks on both ends, then you will also deadlock.

Finally, you seem to assume that send() will always send all the data, and that recv() will always receive one "packet's" worth of data. This is not actually the case for TCP -- it's a stream protocol, and the only guarantee for send() and recv() is that they will transfer AT LEAST one byte, and AT MOST the number of bytes requested in the buffer, before they return.
enum Bool { True, False, FileNotFound };

#3 Drew_Benton   Crossbones+   -  Reputation: 1713

Like
0Likes
Like

Posted 27 March 2011 - 04:30 AM

Another problem with trying to sync a simulation by binding it to the underlying network system is that you put your whole program at the mercy of the connection between the server and client. Right now as you have it, you can't actually use any of the data you are updating until all the data has arrived. You are beginning a data update transaction but allowing it to take as long as needed to complete. What happens if a client drops in the middle of it? The state would be corrupted. Likewise if you are using any of the data outside of this loop in another thread perhaps, the program state would never be in a complete coherent state; it'd always be partial.

As you have it, your whole design is fatally flawed and will not work as you expect it to when you go to add more players. As soon as you introduce threads to handle each connection, your simulation will further get out of sync and have all sorts of race conditions because of how you are not locking access to global data. I'm going to strongly suggest you archive your current version and start a new version taking the following advice into consideration.

First, you will need to redo your entire networking system according to the guidelines hplus mentioned (also read the forum FAQ as it's really helpful!). You don't want to have send/recv functions for specific types of data; just generic buffers of data. The concept of keeping data sends/recvs in order has nothing to do with when you send or receive as much as when you process the data. TCP is a stream protocol so you cannot actually achieve a packet based protocol like UDP is without implementing your own layer on top of it. To accomplish this, you just make use of a state variable to know what to process next. That way, you can do other things while you are waiting for the network data to come and you do not have to do your partial state update transactions as you are now.

Next, you will need to implement a message system to know what data is arriving so you can properly process it. You should never just send strings across TCP without length unless you are actually using a stream protocol (such as HTTP) or you are using fixed sized strings (some games do, still...). The protocol you need is not a stream protocol, but a packet protocol so you must do some extra work. When you do get data across the network, you should also verify the integrity of it. The last thing you want is to code something that is easily exploitable all because you did not implement simple checks in the first place.In the computing world, "this should never happen" should be handled with an exception rather than the silent treatment. If it were to really never happen, you'd not have to think about it first, so if you have to think about it, it could happen (don't ask me how, I've seen it though.)

Finally, consider using an existing network library such as boost::asio that allows you to focus on your program logic more so then the underlying winsock implementation of a decent networking system. boost::asio takes care of a lot of the things you would otherwise have to in regards to the partial send sand receives if you use the API correctly, so that can help you quite a bit if you are not yet familiar with the TCP protocol (once again check out the forum FAQ as it has some really helpful links).

I've written a complete example of what was just mentioned for you to get an idea of one direction you can go. There are many different ways to approach the problem and come up with a solution, but this way is how I'd do it based on what I know so far. It's a lot of code, so take some time to look through the concepts shown rather than the specific implementation as the implementation itself might not be suitable for your task. The concepts though, should be. For reference, take a look at this thread for the visitor related logic and this post for the networking stuff. As a standard disclaimer, code may contain bugs!

main.cpp
Spoiler


visitor.h
Spoiler


visitor.cpp
Spoiler


network.h
Spoiler


network.cpp
Spoiler


And the output would look like this after hitting spacebar then q to exit:
Spoiler


Once you understand the basics of what is going on with the network and visitor logic, the main part you will focus on is the Simulation logic. That is where your applications logic takes place. You feed it inputs and based on state, it reacts accordingly. I tried to keep it simple, so all it uses is some hard coded values, but you should be able to get an idea of how to work with the model.

That is just to give you an idea of another method. It might not be the most ideal model for your simulation. In any case though, I cannot recommend you stick with your current model because it simply won't work (under realistic conditions). You really need to separate the network layer from the business logic layer. Good luck!

"But I, being poor, have only my dreams. I have spread my dreams under your feet; tread softly, because you tread on my dreams." - William Butler Yeats

#4 Chicktopus   Members   -  Reputation: 100

Like
0Likes
Like

Posted 27 March 2011 - 09:16 AM

Hmm, thanks for the code, I'm just trying to get my head around it and figure out if it's appropriate for this task. Fingers crossed!

#5 Chicktopus   Members   -  Reputation: 100

Like
0Likes
Like

Posted 29 March 2011 - 12:49 PM

Ok, I've started again from scratch using UDP (which it wurns out I should have been using anyway).
I'm kind of getting a similar problem where my recvfrom() is only receiving the first couple of characters from the stream, yet that is only when the server attempts to communicate with the client and not vice versa. Curiously enough, the server-side sendto() which corresponds with that recvfrom() returns WSA error 0, which suggests that the client is disconnected.... which it isn't.

Here's my server's send function....


int SendData(SOCKET sd, char* p_sendBuf, sockaddr_in &client, int client_length)
{
	if(sendto(sd, p_sendBuf, (int)strlen(p_sendBuf) + 1, 0, (struct sockaddr *)&client, client_length) != (int)sizeof(p_sendBuf))
	{
		printf("Error sending data: %d\n", WSAGetLastError());
		return 0;
	}
	printf("Sent Data: %d\n", p_sendBuf);
	return 1;
}

...and the client's receive code...

int Socket::ReceiveData(char* p_recvBuf)
{
	if(recvfrom(sd, p_recvBuf, sizeof(p_recvBuf), 0, (SOCKADDR *) &server, (int *) &server_length) < 0)
	{
		printf("Error receiving data: %d\n", WSAGetLastError());
		return 0;
	}
	printf("Received Data: %d\n", p_recvBuf);
	return 1;
}


One thing I did notice, though is that changing 'sizeof(p_recvBuf)' to 4096 increased the amount of data received. Once again, any help would be greatly appreciated.

*Edit* I re-jigged the recvfrom() and it turns out the message I was sending was too large. I've eventually got it working. I think. I'm not sure the correct data is being sent, but that may just be because of the algorithm that creates the data being a bit off.

#6 rip-off   Moderators   -  Reputation: 7660

Like
0Likes
Like

Posted 29 March 2011 - 02:37 PM

UDP is not a connected, reliable stream; it is an unconnected, unreliable datagram protocol. Your code must be able to handle packet loss, packet re-ordering and packet duplication.

#7 hplus0603   Moderators   -  Reputation: 4970

Like
1Likes
Like

Posted 30 March 2011 - 01:04 AM

int SendData(SOCKET sd, char* p_sendBuf, sockaddr_in &client, int client_length)
{
if(sendto(sd, p_sendBuf, (int)strlen(p_sendBuf) + 1, 0, (struct sockaddr *)&client, client_length) != (int)sizeof(p_sendBuf))
[/code]


So, the first problem is that you're asking it to send strlen(p_sendBuf)+1 bytes, but then you're comparing against sizeof(p_sendBuf) bytes.

Given that p_sendBuf is of pointer type, that value will be 4 or 8, depending on your architecture. Meanwhile, strlen(p_sendBuf)+1 will be between 1 and undefined, depending on whether your argument string is properly zero terminated or not.

Second, because you are sending a terminating zero, you will be assuming that that zero is there on the receiving side. This means that someone who wants to crash your receiving end (or, worse, zero-day exploit it) will only have to craft a careful packet that does not including the terminating zero, and whatever you're doing on the receiving end will overflow the buffer. It's better to send the length of the string, and verify that the length of the string is not greater than the amount of data in the packet on the receiving end. Also, if the entire packet is a single string, you could just receive into a buffer that is one bigger than what you tell the system, and then put the terminating 0 at the end of the received data:

  char buf[4096+1];
  int r = recvfrom(sd, buf, sizeof(buf)-1, 0, &addr, &addrSize);
  if (r < 0) {
    // bad socket
  }
  else {
    buf[r] = 0; // zero terminate
    puts(buf); // yay, no buffer overflow!
  }

enum Bool { True, False, FileNotFound };




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS