server implementation using multithreading

[can-mohan]

Hi All,
In below code snippet multithreading is used to take care of multiple requests from client. I beileve creating and detaching thread will eat lots of CPU cycle and it won't improve the CPU utilization as well. Can anybody let me know if thread pool can be used here instead of creating and detaching threads.
kindly share your view on this.

#include <fcntl.h>
     #include <string.h>
     #include <stdlib.h>
     #include <errno.h>
     #include <stdio.h>
     #include <netinet/in.h>
     #include <resolv.h>
     #include <sys/socket.h>
     #include <arpa/inet.h>
     #include <unistd.h>
     #include <pthread.h>

     void* SocketHandler(void*);

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

         int host_port= 8080;

         struct sockaddr_in my_addr;

         int hsock;
         int * p_int ;
         int err;

         socklen_t addr_size = 0;
         int* csock;
         sockaddr_in sadr;
         pthread_t thread_id=0;


         hsock = socket(AF_INET, SOCK_STREAM, 0);
         if(hsock == -1){
             printf("Error initializing socket %d\n", errno);
             goto FINISH;
         }

         p_int = (int*)malloc(sizeof(int));
         *p_int = 1;

         if( (setsockopt(hsock, SOL_SOCKET, SO_REUSEADDR, (char*)p_int, sizeof(int)) == -1 )||
             (setsockopt(hsock, SOL_SOCKET, SO_KEEPALIVE, (char*)p_int, sizeof(int)) == -1 ) ){
             printf("Error setting options %d\n", errno);
             free(p_int);
             goto FINISH;
         }
         free(p_int);

         my_addr.sin_family = AF_INET ;
         my_addr.sin_port = htons(host_port);

         memset(&(my_addr.sin_zero), 0, 8);
         my_addr.sin_addr.s_addr = INADDR_ANY ;

         if( bind( hsock, (sockaddr*)&my_addr, sizeof(my_addr)) == -1 ){
             fprintf(stderr,"Error binding to socket, make sure nothing else is listening on this port %d\n",errno);
             goto FINISH;
         }
         if(listen( hsock, 10) == -1 ){
             fprintf(stderr, "Error listening %d\n",errno);
             goto FINISH;
         }

         //Now lets do the server stuff
           int static count; 
         addr_size = sizeof(sockaddr_in);

         while(true){
             printf("waiting for a connection\n");
             csock = (int*)malloc(sizeof(int));
             if((*csock = accept( hsock, (sockaddr*)&sadr, &addr_size))!= -1){
                 printf("---------------------\nReceived connection from %s\n",inet_ntoa(sadr.sin_addr));
                 pthread_create(&thread_id,0,&SocketHandler, (void*)csock );
                   count++;
                 pthread_detach(thread_id);

             }
             else{
                 fprintf(stderr, "Error accepting %d\n", errno);
             }
         }

     FINISH:
     ;
     }

     void* SocketHandler(void* lp){
         int *csock = (int*)lp;

         char buffer[1024];
         int buffer_len = 1024;
         int bytecount;

         memset(buffer, 0, buffer_len);
         if((bytecount = recv(*csock, buffer, buffer_len, 0))== -1){
             fprintf(stderr, "Error receiving data %d\n", errno);
             goto FINISH;
         }
         printf("Received bytes %d\nReceived string \"%s\"\n", bytecount, buffer);
         strcat(buffer, " SERVER ECHO");

         if((bytecount = send(*csock, buffer, strlen(buffer), 0))== -1){
             fprintf(stderr, "Error sending data %d\n", errno);
             goto FINISH;
         }

         printf("Sent bytes %d\n", bytecount);


     FINISH:
         free(csock);
         return 0;
     }

[rproffitt]

You lost me on those goto's. But your system looks like an echo server. Let's look at priors on good echo servers. Link follows.
https://www.google.com/#q=echo+server+c

I see examples that allow multiple inbound connections without the need to thread like you did. And without a goto either.

[can-mohan]

Hi ,

Thanks for sharing this. I was looking for multithreaded approach and found below code snippet inside accept function from links shared by you and found below piece of code , which smiler to my code except thread detaching is not happening in this case. I don't think so it is performance efficient code in case if you are serving millions of request.

if( pthread_create( &sniffer_thread , NULL ,  connection_handler , (void*) new_sock) < 0)
        {
            perror("could not create thread");
            return 1;
        }

        //Now join the thread , so that we dont terminate before the thread
        //pthread_join( sniffer_thread , NULL);
        puts("Handler assigned");

[rproffitt]

Millions of requests? That means you are not running on a single server but on a Cloud OS with well, a lot more gear. No PC I know will do millions of requests. Look at Facebook's server farm to see a million request server farm.

[can-mohan]

I didn't mean at a time millions of request. thread creation for every request will require lots of cpu cycles as we are creating thread for all the requests from clients and it is not going to improve the cpu utilization.

[JamesCherrill]

Before doing a lot of work, and possibly constructing harder-to-maintain code, maybe you should check your assumptions? How about a trivial program to create and detach 1,0000 threads and see how much CPU that actually takes relative to all the other stuff that's needed to service each request. Shouldn't take more than 1/2 hour, and may save a lot of unecessary work.

In the world of anti-patterns this one is called "premature optimisation".

[jacks009]

server implementation using multithreading is very best practocs for oops langauge.

[can-mohan]

Dear JamesCherril ,
Thanks for sharing your thought on this. Would you advice me to go with this ( create and detach) approach or do you think
thread pool should delegate this task instead of creating thread.