Sending several lines of data through sockets

[thierrypin]

I have this problem:
I have a data structure called DACT, that I convert into a string line and send to another computer. I have a file full of DACTs, and I need to send them all to another computer.
When I send only 100, it works fine, but when I send more, like 500, a bug happens:
It appears that some lines are sent once (checked), but received twice.
I say 'appears' because, after I receive the DACTs, I check if I already have any of the DACTs received, and discard it. For testing, I clear all the DACTs from the receiver and turn the sender on (without any repeated DACT). It happens that the receiver finds some 'repeated DACTs' and discards them. In my last test, I sent 500 and it discarded 18.

Here is the receiver code, omitting error verifications to make it short:

while (true)
{
	recsize = recv(connectFd, (void *) buffer, DACT_LENGHT, 0);

	// Iterates ensure the whole dact is received
	dact_line.append(buffer);
	while (toReceive != recsize)
	{
		toReceive = toReceive - recsize;
		recsize = recv(connectFd, (void *) buffer, toReceive, 0);

		dact_line.append(buffer);
	}
	toReceive = DACT_LENGHT;

	DACT dact;
	str2dact(&dact, dact_line);
	receivedDacts.push_back(dact);

	dact_line.clear();
}

Here is the code that verifies if the DACT has been repeated. Quite simple.

itrecdact = receivedDacts.begin();
for (; itrecdact != receivedDacts.end(); itrecdact++)
{
	repeated = false;

	itdact = dacts->begin();
	for (; itdact != dacts->end(); itdact++)
	{
		if (!strcmp(itdact->getIdDact(), itrecdact->getIdDact()))
		{
			repeated = true;
			break;
		}
	}

	if (!repeated)
	{
		dact2str(&*itrecdact, buffer);
		line_dact = buffer;
		grava_dact(no, &line_dact);
	}
}

And here is the sender code. Also simple.

for (itSend = listaEnvio.begin(); itSend != listaEnvio.end(); itSend++)
{
	dact2str(&*itSend, buffer);
	buffer_length = sizeof buffer;

	bytes_sent = send(sock, buffer, buffer_length, 0);
	cout << "Bytes enviados: " << bytes_sent << endl;
}

I've been weeks looking for the error.

Thank you.

[nezachem]

I am not sure if this is related to your problem, but the receiving loop (lines 7 - 13) looks very suspicious. Is it a copy-paste or you typed it in?

[thierrypin]

I am not sure if this is related to your problem, but the receiving loop (lines 7 - 13) looks very suspicious. Is it a copy-paste or you typed it in?

I made that code myself. Before having this problem, I had a problem in which I didn't receive all lines sent. It was kinda noobish, actually. The standard is that that bug always happened right after this current bug happened.

The logic behind that code is that I keep receiving parts of the line until I get it all. Well, it might be wrong, but I don't see what's wrong in it (the same way I don't see whats wrong with the rest).

[raptr_dflo]

Since your sender-code reads the DACT objects out of a file, and then sends them to the receiver, you should be able to save received DACT objects into a new file (whenever you receive a complete one). When you finish, you can just 'diff' the two files, and that will at least show you which lines are getting duplicated. If you also print a notice when you receive a partial DACT string, before you loop back to get more input, that might shed some more light on what's going wrong and where.

[thierrypin]

Testing it.-

[lcordero]

The problem may be related to the

dact_line.append(buffer);

Assuming dact is a std::string, then the buffer will be copied completly until the first NULL terminator. You should only be appending the number of bytes read, not the entire buffer. Use

string& append ( const char* s, size_t n );

flavor of append instead.

[thierrypin]

OMG. That must be it.
Testing....

[thierrypin]

I tested what you said, and the strangest thing happened.

I used the function you said:

string& append ( const char* s, size_t n );

And the bug continued. Then, to see if I was really receiving repeated lines, i put this in line 19 of the first code I pasted:

cout << "id: " << dact.getIdDact() << endl;

ONLY that. And the code started working. I take it away, and it stops working. I tested this a zillion times, and I am still confused. I also noticed that when this line is NOT there, the executable size is 71.1KB. When the line IS there, the size jumps to 75.5KB.

Then I did some more testing. I just put "cout << endl;" and it works. It has something to do with "endl", because when I put only "cout << "";" or "cout << dact.getIdDact();" it doesn't work.

[lcordero]

Sounds like a timing issue. One other potential problem I see is that you can't assume that send() always succeeds in sending all bytes. You need to do something like you did for the receive loop. The loop could look like this:

for (itSend = listaEnvio.begin(); itSend != listaEnvio.end(); itSend++)
{
	dact2str(&*itSend, buffer);
	buffer_length = sizeof buffer;
        int tot_bytes_sent = 0;

        while (buffer_length > tot_bytes_sent)
        {
  	    bytes_sent = send(sock, &buffer[tot_bytes_sent], buffer_length - tot_bytes_sent, 0);
            tot_bytes_sent += bytes_sent;
        }
	cout << "Bytes enviados: " << tot_bytes_sent << endl;
}

Also, I'm assuming that a DACT converted to a string is fixed length and exactly the size of your buffer.
-lou

[thierrypin]

You assumed right. The DACTs, converted to string, have 142 characters. I use an extra character, \0, to help me control.
By the way, I just noticed I haven't translated this last cout text. It means "Bytes sent: ".
I have to test it. But i don't think this is the problem. For now, my program prints lines for every line received =P. Here, we call this "gambiarra".

[raptr_dflo]

We humorously call bugs that disappear when you add debugging "Heisenbugs", after the Heisenberg Uncertainty Principle (that observing a quantum event affects the outcome of the event). But the truth is, it's usually a very hard-to-find memory error, often a buffer-overrun of some sort, and not really as "unpredictable" as we'd like to believe....

I don't think you're ever copying too much data (e.g., the start of the next DACT) into the buffer, but your code might be a little clearer if re-organized as follows:

DACT dact;
int recsize, total_rec_size, to_receive;
while (True) {
    // 1. reset values
    total_rec_size = 0;
    dact_line.clear();
    // 2. read entire DACT buffer
    while (total_rec_size < DACT_LENGTH) {
        to_receive = DACT_LENGTH - total_rec_size;
        recsize = recv(connectFd, (void *) buffer, to_receive, 0);
        if (recsize <= 0) {
            // connection closed (0) or error (-1)
            break;
        }
        dact_line.append(buffer, recsize);
        total_rec_size += recsize;
    }
    if (total_rec_size < DACT_LENGTH) {
        // broke out of above loop before done, break out of outer one now
        break;
    }
    // 3. convert to structure
    str2dact(&dact, dact_line);
    receivedDacts.push_back(dact);
}

I don't think I've changed anything important, but eliminated a recv() call and an append() call (from the source code, not the execution ... each will still be called the same number of times for the same conditions, just all inside the loop).

[raptr_dflo]

Oh, and if a DACT string is always 142 bytes, and DACT_LENGTH = 142, then if you send the extra null byte (in addition to the 142), then you need to make sure you receive it as well, but I don't think that's your problem or your 2nd received DACT would be corrupted because it's off by one byte, and so on.

Your dact_line string maintains a trailing null byte for you, so you don't have to worry about parsing some infinite string into a DACT (as long as you're appending only the recsize bytes from the buffer onto the end).

[thierrypin]

Oh, and if a DACT string is always 142 bytes, and DACT_LENGTH = 142, then if you send the extra null byte (in addition to the 142), then you need to make sure you receive it as well, but I don't think that's your problem or your 2nd received DACT would be corrupted because it's off by one byte, and so on.

Don't worry about that. DACT_LENGHT is 143.

And thank you. My code really was dirty. I'm cleaning it, the way you showed.
Thank you for the advice. Since its probably a rare memory error, maybe rearranging the code fixes it.

I'll give you some feedback when I finish it.

Thank you a lot.

[thierrypin]

Well, its working putting this.

cout << endl;

Its not a good solution, but fits me for now. So, after a week, I'm closing this topic. Thanks a lot for everybody who helped me =D.