1

Third time I try to ask this question, maybe this time I'll be able to explain my problem better.

I have a multiprocess server with each process doing the accept() (avoiding the Thundering Herd problem with file locking, don't worry). Each process initialize a thread pool (excpet the main one the manage the others). When the accept() succeeds the file descriptor is passed to the thread pool and one of these threads is awakened by a pthread_cond_signal(). After this, the process returns on the file locking waiting to pass through it so it can wait again on the accept(). Meanwhile the thread reads the file descriptor and does its job: reading the HTTP request and serving it in an infinite loop of reading-serving (in order to obtain HTTP persistent-connection). The loop will be broken only if an error occurs or if the timeout expires.

So far so good. But something occurs right after a request is served correctly: in fact, the first request is read and served entirely but when the thread restarts the cycle and enters the read cycle it remains stuck because it reads only few letters like "GE" or "GET", insted of the entire request. If I remove the infinite cycle (for the persisten-connection), each request is served by a different thread and no error occurs!!

This is the reading cycle:

for (;;) {
 ssize_t readn, writen;
 size_t nleft;
 char buff[BUFF_SIZE];
 char *ptr = buff;

 errno = 0;

 nleft = BUFF_SIZE;

 while(nleft > 0) {                             
     //I will read as much as I can using the MSG_DONTWAIT flag making the call non-blocking
     //that means that or the call will succed or it will be closed by the other side
     if ((readn = recv(connsd, ptr, nleft, MSG_DONTWAIT)) < 0) { 

         //If the non-blocking recv fails, it could set errno with one of the following errorcode
         if (errno == EAGAIN || errno == EWOULDBLOCK) {

             //This check has been implemented due to an error that happened several times
             //The buffer was empty even if a new data was sent.
             //This check gives a sort of second chance to the recv.            
             if (strlen(buff) < 10) { 
                 errno = 0;                 //It is important to reset the errno!!
                 continue;
             //If other things occured then I will terminate the string and exit the cicle  
             } else {
                 break;
             }
         // If the conenction has been closed by the client
         } else if (errno == EINTR) readn = 0;
         // If other things occured I will simply shutdown the connection
         else {
             shutdown_sequence(connsd);
             return EXIT_FAILURE;
         }
     // If I read nothing
     } else if (readn == 0) break;

     nleft -= readn;
     ptr += readn;
 }
 buff[strlen(buff)-1] = '\0';
 //request parsing...
 //request serving...
 }

Thanks everyone for the patience!

EDIT1: Just tried using Wireshark in order to see what happen. The first request is read and served correctly, but then I receive "Continuation or non-HTTP Traffic" and [TCP Window Full]... I'm trying this server on a Virtual Machine in Ubuntu 14.04

EDIT2: I tried with a simple loop:

while(nleft > 0) {
        printf("Entering cylce and reading\n");
        fflush(stdout);
        if ((readn = recv(connsd, ptr, nleft, 0)) > 0) { 
            nleft -= readn;
            ptr += readn;
            printf("reading...\n");
            fflush(stdout);
        }
        if (readn == 0) {
            printf("connection closed or nothing more to read\n");
            fflush(stdout);
            break;
        }
        if (readn == -1) {
            printf("error occurred\n");
            fflush(stdout);
            break;
        }
    }

On the terminal I only read:

Entering cylce and reading
reading...
Entering cylce and reading

While Httperf (called with --num-calls=2 --num-conns=1) uses the 50% of the CPU. When I press Ctrl+C to terminate it, the terminal prints:

connection closed or nothing more to read 
buff =
GET /1262662405106.jpg HTTP/1.1
User-Agent: httperf/0.9.0
Host: localhost

EDIT3: In response to David:

while(nleft > 0) {
        printf("I'm going on the read\n");
        fflush(stdout);
        if ((readn = recv(connsd, ptr, nleft, 0)) > 0) { 
            nleft -= readn;
            ptr += readn;
            if (*(ptr-2) == '\r' && *(ptr-1) == '\n') {
                printf("It's an HTTP request\n");
                fflush(stdout);
                break;
            } else  continue;

        } else if (errno == EINTR || readn == 0) {
            break;
        }
    }

It perfectly recognise the first HTTP request beacuse it prints the message. But for the second one it prints "I'm going on the read" once. When I press Ctrl+C the cycle continues indefinitely printing the same message.

EDIT4: So... the problem was in the HTTP response... A mistake with the header and a bad allocation of the string. Thank you, Mr. David!

TheNobleSix
  • 101
  • 10
  • 4
    `strlen(buff) < 10` ... The read has failed. The buffer has no meaningful contents. What is this supposed to do? And why are you using non-blocking reads if you want to block? – usr Aug 24 '15 at 11:14
  • Without MSG_DONTWAIT the recv takes a great amount of time waiting for the HTTP request string. That check instead gives to the recv a second chance and you can't imagine how much it helps. Sometimes instead the request is simply incomplete and the cycle remains stuck in that point! So it means that the recv does not fail but it doesn't read something else. – TheNobleSix Aug 24 '15 at 11:31
  • I tried without MSG_DONTWAIT and it take a lot of time (seconds) just for receiving the first request. – TheNobleSix Aug 24 '15 at 11:44
  • Well, it's not going to be faster with MSG_DONTWAIT. The call just returns without giving you data. Why would that be a good thing? Find out why data is coming in so slowly. All of this elaborate algorithm can be deleted. Just read in a loop and use the value of readn to see how much data came. – usr Aug 24 '15 at 12:01
  • My interpretation of "TCP Window Full" is that you are not reading all of the data. Might be true because you break out of the read loop under conditions which I don't understand. Why are you exiting the read loop? – usr Aug 24 '15 at 12:05
  • Without MSG_DONTWAIT that string or the request does not arrive (so the thread remains blocked in that point) or arrives after many seconds. How can I find out why data comes so slowly? I did the "simple loop" that you said at the very beginning but we had these problems so we created this elaborate algorithm that works... – TheNobleSix Aug 24 '15 at 12:07
  • So if it works why are you here? You need to understand the problem instead of fixing the symptoms. Write a simple read loop, test it and post code and results here. I don't know how else to help you since you are not willing to be corrected in your wrong assumptions. – usr Aug 24 '15 at 12:09
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/87778/discussion-between-thenoblesix-and-usr). – TheNobleSix Aug 24 '15 at 12:10
  • Overthink calling `strlen()` on a buffer containing garbage, that is not being initialise properly. – alk Aug 24 '15 at 16:16
  • It was strlen() == 0 at the beginning. The secondo chance worked very well and 10 was a random number beacuse GE or GET was 2 and 3 character long. – TheNobleSix Aug 24 '15 at 16:22
  • 1
    Do not break on `EAGAIN`, but just `continue`, ever. Also you might like to read `recv()`'s documentation closely. TCP provides a data stream. It may return data in chunks down to a size of one byte for each call, indepedantly to how many calls to `send()` had benn used. – alk Aug 24 '15 at 17:10
  • 1
    `strlen()` expects a `0`-terminated `char` array, a C-"string". If the argument does not refer to a `0`-terminated C-"string", `strlen()` reads beyond its arguments boundaries and with this invokes undefined bahaviour. The code as it stands does not guarantee `buffer` to be ^0`-terminated. For your reference: http://man7.org/linux/man-pages/man3/strlen.3.html – alk Aug 24 '15 at 17:14
  • 1
    All in all the code shown it doubted to receive any complete HTTP packets being parseable on a realiable base, as the code does not take any efforts to receive complete HTTP commands. HTTP commands are (mostly) delimited by CR-LF sequence(s). So those need to be taken into account by the receiver, which the code does not. Start-over reading the HTTP specs. You want to start here: http://tools.ietf.org/html/rfc7230 and continue up to RFC 7239. – alk Aug 24 '15 at 17:21
  • @alk I apprieciate your help! Try to read the chat discussion, maybe there is something that can help you understand what I faced. – TheNobleSix Aug 24 '15 at 17:59
  • @alk strlen(buff) works perfectly even if the string is not 0-terminated: I receive "GET" and strlen(buff) is 3. What you said is not present in the link you posted. – TheNobleSix Aug 25 '15 at 10:00
  • Continue reading here http://tools.ietf.org/html/rfc7231 http://tools.ietf.org/html/rfc7232 http://tools.ietf.org/html/rfc7233 http://tools.ietf.org/html/rfc7234 http://tools.ietf.org/html/rfc7235 http://tools.ietf.org/html/rfc7236 http://tools.ietf.org/html/rfc7237 http://tools.ietf.org/html/rfc7238 http://tools.ietf.org/html/rfc7239 – alk Aug 25 '15 at 10:10
  • How do you discover which sockets to read and when? – David Schwartz Aug 25 '15 at 10:10
  • 1
    "*`strlen(buff)` works perfectly even if the string is not 0-terminated*" No, it won't. And if it looks as if does this is just (bad?) luck. There probably just was a random `0` just in the right place at the right moment. – alk Aug 25 '15 at 10:11
  • @DavidSchwartz There is only one socket to read. The one passed to the thread when the accept() succeed. I suppose that it should be ready to be read just after I receive it (?). – TheNobleSix Aug 25 '15 at 10:20
  • `(readn == EINTR || readn == -1)` That can't be right. Maybe you want `(readn == 0) || (errno != EINTR)`? – David Schwartz Aug 25 '15 at 11:22

1 Answers1

1

If you're going to do non-blocking I/O and don't want to burn the CPU at 100%, you have to have some place in your code where you wait for data to arrive. You have no such code, so you burn the CPU at 100% spinning tightly while you wait for data to arrive. It sounds like you want blocking I/O. (Start by getting rid of MSG_DONTWAIT.)

Also, don't use strlen to figure out the length of something that's not a string. If you need to know how many bytes were received, keep track of it yourself.

in fact, the first request is read and served entirely but when the thread restarts the cycle and enters the read cycle it remains stuck because it reads only few letters like "GE" or "GET", insted of the entire request.

If you haven't read the entire request, call the read function again until you have an entire request. Use a blocking read.

Basically:

  1. Do a blocking read into our request buffer, after any data already in the buffer.
  2. Did I get an error or is the connection closed? If so, stop.
  3. Do I have a complete request according to the HTTP protocol? If not, go to step 1.
  4. Process the request, send the response.
  5. Go to step 1.
David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • The CPU does not burn at 100%. Never. The thread does not enter in that cycle until awakened by the control process after a connection has been established. When i do non-persistent-connection (1000 calls with Httperf for example) each process consume 1% of the CPU, perfectly balanced. For the
    strlen I will find a solution.
    – TheNobleSix Aug 25 '15 at 10:39
  • @TheNobleSix: With "*... burn the CPU at 100% spinning tightly while you wait for data to arrive*" David is referring to the receiver's while-loop when operation in non-blocking mode, as the code does when passing `MSG_DONTWAIT` to `recv()`. – alk Aug 25 '15 at 10:46
  • @TheNobleSix Create a connection to your server and then send just a single byte to it. Watch what happens. Where do you think you are waiting when a connection has been established but little or no data has been received? – David Schwartz Aug 25 '15 at 10:48
  • "Do a blocking read into our request buffer, after any data already in the buffer." What do you mean with this? – TheNobleSix Aug 25 '15 at 10:56
  • @TheNobleSix I mean that the adress you pass to `read` should be the first unused byte in the buffer. (Just like your example code already does.) – David Schwartz Aug 25 '15 at 10:57
  • You have no code to handle the normal close of the socket. No code handles the `readn == 0` case. – David Schwartz Aug 25 '15 at 11:23
  • @DavidSchwartz Nothing changed. – TheNobleSix Aug 25 '15 at 11:26
  • Okay, so what is `readn` returning in the problem case? What is the value of `errno`? (If `readn` is -1.) – David Schwartz Aug 25 '15 at 11:29
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/87880/discussion-between-thenoblesix-and-david-schwartz). – TheNobleSix Aug 25 '15 at 11:34