0

I am having a hard time figuring out a bug in my TCP client-server app. The problem I am facing: in my recv function do-while loop, if the condition is bytes > 0, the function hangs forever. Replacing that with bytes == NMAX, everything works fine, UNLESS NMAX is equal to 1. A few side notes: doing a single send-recv works fine, but trying to do a send-recv and then recv-send hangs forever. NMAX is a constant set to 4096 by default. Server is ran first, then the client. This is my send function:

ssize_t sendData(const std::string data, int fd)
{
    ssize_t total = data.length(), bytes, sent = 0;

    do
    {
        ssize_t chunk = total > NMAX ? NMAX : total;
        bytes = send(fd, data.c_str() + sent, chunk, 0);

        if (bytes == -1)
        {
            throw std::system_error(errno, std::generic_category(), "Error sending data");
        }

        total -= bytes;
        sent += bytes;
    } while (total > 0);

    return sent;
}

This is my recv function:

std::string recvData(int fd)
{
    ssize_t bytes;
    std::string buffer;

    do
    {
        std::vector<char> data(NMAX, 0);
        bytes = recv(fd, &data[0], NMAX, 0);

        if (bytes == -1)
        {
            throw std::system_error(errno, std::generic_category(), "Error receiving data");
        }

        buffer.append(data.cbegin(), data.cend());
    } while (bytes > 0); // Replacing with bytes == NMAX partially fixes the issue, why?

    return buffer;
}

This is the client's main function:

std::cout << "Sent " << sendData(data) << " bytes\n";
std::cout << "Received: " << recvData() << "\n";

And this is the server's main function:

std::cout << "Received: " << recvData(client) << "\n";
std::cout << "Sent " << sendData("Hello from the server side!", client) << " bytes\n";
Cata Cata
  • 111
  • 1
  • 6
  • 1
    This question's shown code does not meet stackoverflow.com's requirements for a [mre]. This means it's unlikely that anyone here can conclusively answer the question; but only guess at the most. You should [edit] your question to show a minimal example, no more than one or two pages of code (the "minimal" part), that everyone else can cut/paste, compile, run, and reproduce the described issue (the "reproducible" part) ***exactly as shown*** (this includes any ancillary information, like the input to the program). See [ask] for more information. – Sam Varshavchik Sep 05 '20 at 12:22
  • @SamVarshavchik hope it's more clear now – Cata Cata Sep 05 '20 at 12:41
  • Can you answer the following question: can everyone in the world cut and paste what's shown in the question, ***exactly as shown***, then compile, link, execute, and reproduce this problem? Create a new, empty file. Cut/paste only what's in the question. Do you still have the same problem with the compiled code? Unless you can answer "yes" to this question, it is not a [mre]. I'm pretty sure I see the problem, but it's only a guess, and I'm not going to waste a bunch of time typing it up and explaining, only to find out that the real problem is something that's not shown, here. – Sam Varshavchik Sep 05 '20 at 12:45
  • 2
    `recv` usually blocks, waiting for more data to arrive, until a) more data does arrive (in which case it returns the number of bytes read), b) the connection is closed (in which case it returns 0) or c) an error occurs (in which case it returns -1). Apparently, your client has read all the data available, and is waiting for more to arrive; and your server never closes the connection. – Igor Tandetnik Sep 05 '20 at 16:34
  • 1
    Changing the condition to `bytes == NMAX` helps because the loop is terminated whenever `recv` happens to receive fewer than `NMAX` bytes. But when `NMAX == 1`, it's impossible to receive fewer than `NMAX` bytes. – Igor Tandetnik Sep 05 '20 at 16:35

1 Answers1

1

The problem with your program is that the receiving side does not know how many bytes to receive in total. Therefore it will just endlessly try to read more bytes.

The reason why it "hangs" is that you perform a blocking system call (recv) which will only unblock if at least 1 more byte had been received. However since the peer does not send more data this will never happen.

To fix the issue you need to have a proper wire-format for your data which indicates how big the transmitted data is, or where it starts and ends. A common way to do this is to prefix data with it's length in binary form (e.g. a 32bit unsigned int in big endian format). Another way is to have indicators inside the data that indicate it's end (e.g. the \r\n\r\n line breaks in HTTP).

Btw: Your send function is not ideal for cases where data.length() == 0. In this case you perform a send system call with 0 bytes - which is rather unnecessary.

Matthias247
  • 9,836
  • 1
  • 20
  • 29
  • Alternatively use `MSG_DONTWAIT ` in the flags and handle `EAGAIN` etc - use the socket in a non-blocking fashion. See the MAN pages https://man7.org/linux/man-pages/man2/recv.2.html – Den-Jason Sep 06 '20 at 01:14
  • Also see this thread vis-a-vis setting socket timeouts https://stackoverflow.com/questions/4181784/how-to-set-socket-timeout-in-c-when-making-multiple-connections – Den-Jason Sep 06 '20 at 01:16
  • How would that help? If the socket returns `EAGAIN` at some point the caller still dosn't know whether this is the end of the current message or whether the next IP packet simply had been delayed, but that there is still data to be received. The only alternative is to shutdown the socket on the writer end. Then the reading side will read `0` - which can be interpreted as the end of the message. Doesn't work however if more than one message should be exchanged. Then any kind of delimeter is mandatory. – Matthias247 Sep 06 '20 at 03:47
  • You simply implement a message builder. `recv` can be broken anyway by signals (and yes you have to check for that). Also when you have blocking "header+data" it's possible (but unlikely) that the data becomes truncated, and the receiver ends up either waiting until the next header is received and the synchronisation is lost. Definitely need a sync-mark in the header, as well as the use of CRCs at the end of header and the end of message if you want to be 100% sure. – Den-Jason Sep 06 '20 at 11:09