trouble converting code from udp to tcp.

[drewdaman]

hi everyone,

i have yet another problem! i had some code that works fine over udp. now i'm trying to adapt that code to work over tcp. but it doesn't work.

originally, i had a fucntion that sends a file over udp adn another function that receives this file. now i want to do this over tcp. i think my problem is in my receiving function, because it just keeps waiting to receive more data. the way i had this set up when i was using udp was to have a special string to see when the transfer was complete. i dont' think that it ever gets the special string. i am pasting my code below. Just to be clear, i think the error occurs because the line


if(!comparestr(goodbyemsg,temp, strlen(goodbyemsg))){


never evaluates to false and never gets into the else part. it always works when i do this over udp. in fact, i was surprised that i never lost a packet while doing things over udp! maybe because it was just between two computers on the same network. here is the code (i have left teh udp code in there as comments so you can take a look if you like- that code works):

void Server::receivefile(){

//int buffSize=1000;
char * temp=new char [buffSize];
int nLen;
bool done=false;
fstream q("test.jpg", ios::out|ios::binary);
nLen = sizeof(SOCKADDR);
unsigned char aa;

while(!done){

/* int nRet = recvfrom(theSocket, // Bound socket
temp, // Receive buffer
buffSize, // Size of buffer in bytes
0, // Flags
(LPSOCKADDR)&saServer, // Buffer to receive client address
&nLen); // Length of client address buffer
*/
recv(theClient,temp,buffSize,0);
if(!comparestr(goodbyemsg,temp, strlen(goodbyemsg))){
for (int i=0; i<buffSize;i++){// && temp[i]!=EOF; i++){
aa=(unsigned char)temp[i];
q<<aa;
//if (temp[i]==EOF) q<<temp[i];
}
}
else{
done=true;
}

}//close while loop
q.close();
}

bool Server::comparestr(char * a, char *b, int length){
bool ret=true;
int i=0;
while (i<length && ret){
if (a[i]!=b[i]){
ret=false;
}
i++;
}

return ret;
}


i had read once that tcp sort of has a mind of its own. it sort of understands what you are trying to send and sometimes does not send the buffers exactly as you tell it to. it seems a little bit odd to me.. but do you think something like this might be my problem? if not, do you see what the problem could be? because if the special string does not start at position 0 in the received buffer, my code will not work.

thanks!
drew.

[Tomcat]

UDP is packet oriented, so if you send the "goodbyemsg" as one packet it will also be received as one packet (that contains the complete goodbyemsg and nothing but the goodbyemsg).

TCP is stream oriented, so you cannot assume that the "goodbyemsg" is the only thing you get in the buffer when calling recv(), or even that you get the entire goodbyemsg in the same call. You more or less have to treat the received data as if it arrives a single byte at a time.

Starting the transmission with the length of the file that then follows is probably a better idea, then you can simply recv() until you know that the file is complete (remember that TPC "guarantees" that no data is lost).

[drewdaman]

hi.. thanks for your response(s) tomcat.

then this doesn't seem like a great way to figure out that the file is done...

how would you recommend doing this? using a filesize or something maybe? ie the first message should be sent from the file sender and should contain the size of the file (in bytes) ... the receiver notes this and stops writing to the new file when it has written that many bytes.. any other suggestions?

thanks.

[Mathew Joy]

While switching over to tcp, you need to know a few basic properties of TCP and the most important one is that it never preservers message boundaries(as what is said by Tom). The way you've written the function does not take into account this property. You defenitly need to do some reading about TCP and winsock. Follow the link in my signature and look at the network FAQ. Do some slow and carefull reading; some basic priciples are discussed.

What you have to keep in mind is that the message (or whatever) can be in 2 recvs().

[MikeAThon]

...how would you recommend doing this? using a filesize or something maybe? ie the first message should be sent from the file sender and should contain the size of the file (in bytes) ... the receiver notes this and stops writing to the new file when it has written that many bytes.. any other suggestions?....
That's a good technique for sending a file by TCP. See this article: "Network Transfer Of Files Using MFC's CSocket Class', at http://www.codeproject.com/internet/SocketFileTransfer.asp

Although the article is written for CSocket, it should be easy to adapt to plain Winsock calls.

Mike

[drewdaman]

thanks for the responses guys. but i'm still having trouble.

i have changed the code to send the file size and use that to control how the file is transfered. the original file and the transfered file are exactly the same size in bytes. i verified this using a little c++ program i found on the web.. not using what windows says.. because with what windows says 11.41kb=11.42kb=11.4kb. and we need to know that it is the same size down to the last byte. but when i compare the files (using comp in a dos window), there are several differences.

but now, although the files are exactly the same size, when i open the transfered file, it is different. i'm testing by transfering a jpeg. the pictures are different.. the transfered one looks like the original but with someone shining a really really bright light on it. and you can't really make out much. you couldn't tell what it is without having seen the original.

i'm guessing i'm having the same trouble as before. i guess the packet with the size of the file gets merged with the data. do you have any suggestions on how i can fix this? i tried adding a garbage send and receive over teh same socket to see if this is my problem... and it seems it is because the picture i received was different from teh earlier (incorrect one)... so something changed, but it didn't get fixed. i hope that is not too confusing to read!

here is the code.

sender:


void CamClient::sendfile(){
//int buffSize=1000;
struct stat results;
long size = 0;
unsigned char *pBuffer = 0;
char *sendBuffer=new char[buffSize];

//put the entire file in a buffer called pBuffer.
if (stat("a.jpg", &results) == 0)
size = results.st_size;
cout<<"SIZE IS: " <<size<<endl;
ifstream myFile ("a.jpg", ios::in | ios::binary);

pBuffer = new unsigned char[size];

myFile.read(pBuffer, size);


myFile.close();
int nRet=0; int i=0; int j=0;

//send the size of the file to the server.

itoa(size,sendBuffer,10);
strcat(sendBuffer,",");
send(tcpSocket,sendBuffer,buffSize,0);
char *ll=new char[2]; //this is the garbage packet
recv(tcpSocket,ll,2,0); //this is the garbage packet

bool done=false;
while (i<=size && !done){

if (j==buffSize|| i==size){
if (i==size) done=true;
nRet=send(tcpSocket,sendBuffer,buffSize,0);



j=0;
memset(sendBuffer,0,buffSize);
}
else{
sendBuffer[j]=pBuffer[i];
j++;i++;
}



}


receiver:

void Server::receivefile(){


char * temp=new char [buffSize];
int nLen;
bool done=false;
fstream q("test.jpg", ios::out|ios::binary);
nLen = sizeof(SOCKADDR);
unsigned char aa;
//receive the filesize
recv(theClient,temp,buffSize,0);

int filesize=parsefilesize(temp);
int counter=0; //to count number of bytes written
send(theClient,"hi",2,0); //garbage packet
int nopackrecv=0;
while (counter<filesize && (nopackrecv)<ceil(((double)filesize/(double)buffSize)-1)){
recv(theClient,temp,buffSize,0);
nopackrecv++;
for (int i=0; i<buffSize;i++){// && temp[i]!=EOF; i++){
aa=(unsigned char)temp[i];
q<<aa;

counter ++;
}
}//close while.
//receive last packet
//cout<<"nopackrecv is: "<<nopackrecv<<endl;
recv(theClient,temp,buffSize,0);
for (int j=0;j<(filesize-counter);j++){
aa=(unsigned char)temp[j];
q<<aa;

}
}//end function receivefile



I really don't see any other possible error than what i described earlier... is there some way to flush stuff out? like tell it, ok, send everythign now and everything i send after t his separately? if not, how do i deal with this issue?

thanks,
drew.

ps- i dont' have access to my code right now, but was just checking here... so i can't try this idea out immediaetly... do you think if i were to make a separate function to exchange the size of the file my problem would be solved? does returning from a fucntion force everything that is waiting to be sent to actually be sent?

[MikeAThon]

You've made the classic error in TCP communications: you assume that a call to Winsock's recv function will return exactly the number of bytes requested for the buffer. It won't.

The specs are clear on this point: recv's return value will tell you how many bytes were actually transferred into the buffer, and that value can be anything between 1 and the number of bytes requested. Your code must test the value actually received, and account for the fact that it might be different than expected.

Your code doesn't account for this difference, and merely assumes that all bytes requested will be returned by recv. So, although the size of the file is correct, in fact, the contents of the file are corrupted for each time that recv returns fewer bytes than expected.

The article I sent you to tells you about this, and includes code to accommodate "mismatches", i.e., situations where recv gives you fewer bytes than requested for your buffer. Study the code more carefully.

Note that this situation can occur anytime you recv more than a single byte. So, even the recv of the integer value representing the size of the file (four bytes) can be corrupted.

Also note that the problem can occur on the sending side too, for non-blocking sockets.

Mike

[Mathew Joy]

One of the things you have to learn

Always check the return value of the winsock function calls.

This is very important.

[drewdaman]

hi guys,

thanks for the responses.

yeah! that seems like a totally rookie mistake! i am stilll a rookie :P i haven't done much with tcp before.. been using udp as i mentioned earlier.

i will try to fix it up on monday... if i run into further problems i will get back to you guys!

thanks again!

ps. Mathew, would it be possible for you to put up the multithreaded server code? i had mentioned it in another post, i'm not sure if you saw it. you could either put it up here or email it to me at dhruv22@gmail.com.. would be much appreciated.. thanks!

[drewdaman]

hi guys,

i did what you suggested.

on the sending end, i pritned the number of bytes being sent. ie. i printed teh value being returned by send. this was as expected.

on the receiving end, there were some problems. i printed the number of bytes being received and the last error. the first packet seems to have been received fine. but after that, number of bytes received was -1. and the error i got was 10054. From MSDN, error 10054 is:


Connection reset by peer.
An existing connection was forcibly closed by the remote host. This normally results if the peer application on the remote host is suddenly stopped, the host is rebooted, the host or remote network interface is disabled, or the remote host uses a hard close (see setsockopt for more information on the SO_LINGER option on the remote socket). This error may also result if a connection was broken due to keep-alive activity detecting a failure while one or more operations are in progress. Operations that were in progress fail with WSAENETRESET. Subsequent operations fail with WSAECONNRESET.


The output of my program is:


C:\abc\qwerty\demo\camServer\Debug>camserver 8888
Bytes received: 1000
LAST ERROR: 0
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
Bytes received: -1
LAST ERROR: 10054
nopackrecv is: 11
Bytes received after while loop: -1
LAST ERROR: 10054


i am really at a loss! why is the connectin being reset? i'm copying my sending and receiving code here. i am also attaching the entire project in case you would like to take a look.

Receving code (this is the code with the error):

void Server::receivefile(){

//int buffSize=1000;
char * temp=new char [buffSize];
int nLen;
bool done=false;
fstream q("test.jpg", ios::out|ios::binary);
nLen = sizeof(SOCKADDR);
unsigned char aa;
//receive the filesize
recv(theClient,temp,buffSize,0);
//cout<<"temp is: "<<temp<<endl;
int nRet;
int filesize=parsefilesize(temp);
int counter=0;
nRet=send(theClient,"hi",2,0);
int nopackrecv=0;
while (counter<filesize && (nopackrecv)<ceil(((double)filesize/(double)buffSize)-1)){
nRet=recv(theClient,temp,buffSize,0);
cout<<"Bytes received: "<<nRet<<endl;
nRet=WSAGetLastError();
cout<<"LAST ERROR: "<<nRet<<endl;
nopackrecv++;
for (int i=0; i<buffSize;i++){// && temp[i]!=EOF; i++){
aa=(unsigned char)temp[i];
q<<aa;
//if (temp[i]==EOF) q<<temp[i];
counter ++;
}
}//close while.
//receive last packet
cout<<"nopackrecv is: "<<nopackrecv<<endl;
nRet=recv(theClient,temp,buffSize,0);

cout<<"Bytes received after while loop: "<<nRet<<endl;
nRet=WSAGetLastError();
cout<<"LAST ERROR: "<<nRet<<endl;
for (int j=0;j<(filesize-counter);j++){
aa=(unsigned char)temp[j];
q<<aa;
//if (temp[i]==EOF) q<<temp[i];
//counter ++;
//cout<<"1\n";
}
}//end function receivefile


Sending code (seems to be fine).

void CamClient::sendfile(){

//int buffSize=1000;
struct stat results;
long size = 0;
unsigned char *pBuffer = 0;
char *sendBuffer=new char[buffSize]; //USED TO BE UNSIGNED!
//BUT WORKS LIKE THIS TO COPY FILE.
//put the entire file in a buffer called pBuffer.
if (stat("a.jpg", &results) == 0)
size = results.st_size;
cout<<"SIZE IS: " <<size<<endl;
ifstream myFile ("a.jpg", ios::in | ios::binary);

pBuffer = new unsigned char[size];

myFile.read(pBuffer, size);
//at this point, the entire file to be sent is in pBuffer.

myFile.close();


int nRet=0; int i=0; int j=0;

itoa(size,sendBuffer,10);
strcat(sendBuffer,",");
send(tcpSocket,sendBuffer,buffSize,0);
char *ll=new char[2];
recv(tcpSocket,ll,2,0);

bool done=false;
while (i<=size && !done){

if (j==buffSize|| i==size){ //ie. buffer is full or file is done
//(even if buffer is not full)
if (i==size) done=true;
//send

nRet=send(tcpSocket,sendBuffer,buffSize,0);
cout<<"Bytes sent: "<<nRet<<endl;
j=0;
memset(sendBuffer,0,buffSize);
}
else{
sendBuffer[j]=pBuffer[i];
j++;i++;
}



}

// nRet=send(tcpSocket,goodbyemsg,strlen(goodbyemsg),0);

}


i might also mention that my test jpeg is 11751 bytes.

thanks for all the help guys.

in the attachement, there are two folders if you extract... all the files are in the correct folders. (so, please extract!)

thanks again!

ps. this might also be the source of my original error: the fact that i cannot test it on my local machine. i just don't understand it! :(

[drewdaman]

hi guys...
yet another update... quite strange! i sort of fixed the error 10054 problem... but its a terrible way to fix it!

ok.. here is what i did... in my camClient main method, i put in an infinite loop. so my main mehtod looks like this:

#include "CamClient.h"

void main(int argc, char **argv)
{

...... //there was some useless code here... (to run the camClient type in "camClient aa 22")

CamClient theClient;
theClient.TCPinitial(2);
theClient.sendfile();

//this is what i added!!!!!!!!!
while (true){};
}



So, this solves teh error 10054 problem. it solves it, but it is a horrible way to solve it! any suggestions on a better way to fix it?

i am receiving the picture correctly now and here is the working code.. needs to be cleaned up (a lot!) but it works....

sender:

void CamClient::sendfile(){

//int buffSize=1000;
struct stat results;
long size = 0;
unsigned char *pBuffer = 0;
char *sendBuffer=new char[buffSize]; //USED TO BE UNSIGNED!
//BUT WORKS LIKE THIS TO COPY FILE.
//put the entire file in a buffer called pBuffer.
if (stat("a.jpg", &results) == 0)
size = results.st_size;
cout<<"SIZE IS: " <<size<<endl;
ifstream myFile ("a.jpg", ios::in | ios::binary);

pBuffer = new unsigned char[size];

myFile.read(pBuffer, size);
//at this point, the entire file to be sent is in pBuffer.

myFile.close();


int nRet=0; int i=0; int j=0;

itoa(size,sendBuffer,10);
strcat(sendBuffer,",");
send(tcpSocket,sendBuffer,buffSize,0);
char *ll=new char[2];
recv(tcpSocket,ll,2,0);

bool done=false;
while (i<=size && !done){

if (j==buffSize|| i==size){ //ie. buffer is full or file is done
//(even if buffer is not full)
if (i==size) done=true;
//send

nRet=send(tcpSocket,sendBuffer,buffSize,0);
cout<<"Bytes sent: "<<nRet<<endl;
j=0;
memset(sendBuffer,0,buffSize);
}
else{
sendBuffer[j]=pBuffer[i];
j++;i++;
}



}

receiver:


void Server::receivefile(){

//int buffSize=1000; -this is a class variable in my code, don't need it.
char * temp=new char [buffSize];
int nLen;
bool done=false;
fstream q("test.jpg", ios::out|ios::binary);
nLen = sizeof(SOCKADDR);
unsigned char aa;
int nRet;
//receive the filesize
nRet=recv(theClient,temp,buffSize,0);

int filesize=parsefilesize(temp);
int counter=0;
nRet=send(theClient,"hi",2,0);

int nopackrecv=0;
while (counter<filesize){// && (nopackrecv)<ceil(((double)filesize/(double)buffSize)-1)){
nRet=recv(theClient,temp,buffSize,0);
//cout<<"Bytes received: "<<nRet<<endl;
//nRet=WSAGetLastError();
//cout<<"LAST ERROR: "<<nRet<<endl;
nopackrecv++;
for (int i=0; i<nRet;i++){
aa=(unsigned char)temp[i];
q<<aa;
//if (temp[i]==EOF) q<<temp[i];
counter ++;
}
}//close while.
//receive last packet

}//end function receivefile


but i still need a better way to deal with error 10054.
thanks!

[Mathew Joy]

How do you expect the connection to be alive when the application is closed? What you need is graceful closure. One way is to wait for the client to message back that it has got the file. Another but bit more complicated way is close the connection in a graceful way. Once the send is completed, call shutdown() with SD_SEND as the how parameter, call recv() and when you get 0 as the return value call closesocket(). On the client side, loop recv() untill you get 0 indicating the socket is gracefully closed. Then you can call closesocket().

[drewdaman]

thanks for replying.
well.. i thought once data was sent, it should be all good and the receiving end would deal with it then.. i could not have been more wrong! for now, i think i will leave my while loop there.. later on i think i will deal with it with a blocking recv (a request to receive data from this particular source). but i need to be able to multithread to get there! please see my other post at http://www.codeguru.com/forum/showthread.php?t=324686 :)

thanks!

[Mathew Joy]

well.. i thought once data was sent, it should be all good and the receiving end would deal with it then.. i could not have been more wrong!

It wouldn't have happened it you read the faqs more carefully. Anyway good luck.

[MikeAThon]

It wouldn't have happened it you read the faqs more carefully. Anyway good luck.

Mathew,

I don't think that the FAQ covers this situation, at least not exactly. The FAQ You're probably referring to is "When does 'send()' return?" at http://www.codeguru.com/forum/showthread.php?s=&threadid=296875 and it says the following:
...In general, a successful 'send()' simply means that the data has been passed to the lower winsock layer for processing. Since TCP guarantees delivery of data, it can be assured that the data that is handed down for processing will be delivered, as long as the connection is not abnormally terminated and there is no out-of-resource condition on the receiving side.
Now, maybe to you the phrase "the connection is not abnormally terminated " means that the application stays alive, but I think most people reading the FAQ would focus on the first part, where we have stated that "TCP guarantees delivery of data".

Maybe it's worthwhile to modify the FAQ, such as by stating that "... as long as the connection is not abnormally terminated (such as by closing the application without a graceful shutdown/closesocket sequence) and there is no out-of-resource condition on the receiving side." Just a suggestion.

Here, in the OP's situation, I think the sending side merely needs a graceful shutdown, through calls to shutdown() and closesocket(). I don't think he needs to implement any mechanism to ensure that "the send is completed" (as you indicated in a previous post) before initiating calls to shutdown/closesocket, and rather I think that these functions can be called immediately after his call to send returns. See "Graceful Shutdown, Linger Options, and Socket Closure" at http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winsock/winsock/graceful_shutdown_linger_options_and_socket_closure_2.asp

Mike

[drewdaman]

thanks for all the help guys.

yeah, i will have to look into doing a graceful shutdown. but first i think i will focus on getting my server to be multithreaded! i will of course, have to do the graceful shutdown eventually..