Async chat server/client with select()

hello out there.

im trying to create a simple chat server/client with select method in C using Visual Studio 2010.
Clients will connect to the server and the server will send the message of each client to the rest.

We talk about a console app.

and here is the code.

Server:

Code:

---

#include <WinSock2.h>
#include <stdio.h>
#include <time.h>



main()
{

        SOCKET                        ListeningSocket;
        SOCKET                        AcceptSocket;

        SOCKADDR_IN                ServerAddr;
        SOCKADDR_IN                ClientAddr;

        WSADATA                        wsaData;

        const unsigned short PORT = 4444;

        FD_SET                        fdread;
        FD_SET                        BackUpfdread;
        FD_SET                        fdwrite;
        FD_SET                        BackUpfdwrite;

        int                                maxDescriptor;
        SOCKET                        SocketArray[20];
        int                                index = 0;
        int                                selectResults;
        int                                i,k;
        int                                clientAddrSize;
        int                                RecvBytes;
        int                                SentBytes;

        char                        SentBuff[500];
        char                        RecvBuff[500];

        struct        timeval timeout;


        // Initialize Winsock2.2
        WSAStartup(MAKEWORD(2,2),&wsaData);

        // Initialize Listening Socket
        ListeningSocket = socket(AF_INET,SOCK_STREAM,0);


        // Initialize ServerAddr
        ServerAddr.sin_family = AF_INET;
        ServerAddr.sin_addr.s_addr = htonl(INADDR_ANY);
        ServerAddr.sin_port = htons(PORT);

        // Bind the ServerAddr with ListeningSocket
        bind(ListeningSocket,(SOCKADDR *)&ServerAddr,sizeof(ServerAddr));

        // Listening Socket
        listen(ListeningSocket,5);

        FD_ZERO(&fdread);
        FD_ZERO(&BackUpfdread);
        FD_ZERO(&fdwrite);
        FD_ZERO(&BackUpfdwrite);

        // Asign ListeningSocket at fdread
        FD_SET(ListeningSocket,&fdread);

        maxDescriptor = ListeningSocket;

        SocketArray[index] = ListeningSocket;
        index++;

        timeout.tv_sec = 0;
        timeout.tv_usec = 0;

        // Main loop starts here
        for(; ;)
        {
               
                BackUpfdread = fdread;
                BackUpfdwrite = fdwrite;
                selectResults = select(maxDescriptor+1,&BackUpfdread,&BackUpfdwrite,NULL,&timeout);
                //printf("server select() OK\n");

                if(selectResults == -1)
                {
                        printf("Select() error");
                        WSACleanup();
                        return 0;
                }


                for(i=0;i<=index-1;i++)
                {
                        //printf("%d\n",SocketArray[i]);
                        if(FD_ISSET(SocketArray[i],&BackUpfdread))
                        {
                                if(SocketArray[i] == ListeningSocket) // we have a new connection
                                {
                                        clientAddrSize = sizeof(ClientAddr);
                                        AcceptSocket = accept(ListeningSocket,(SOCKADDR *)&ClientAddr,&clientAddrSize);

                                        // Add the newest accepted socket to the fdread and fdwrite sets.
                                        FD_SET(AcceptSocket,&fdread);
                                        FD_SET(AcceptSocket,&fdwrite);

                                        // Add the newest accepted socket into SocketArray
                                        SocketArray[index] = AcceptSocket;
                                        index++;

                                        // keep track of the maxDescriptor.
                                        if(AcceptSocket > maxDescriptor)
                                        {
                                                maxDescriptor = AcceptSocket;
                                        }

                                        printf("New connection from %s on socket %d\n", inet_ntoa(ClientAddr.sin_addr), AcceptSocket);

                                }else{ // That means that the socket is not from a new conection and has something sent.

                                        memset(RecvBuff,0,sizeof(RecvBuff));
                                        RecvBytes = recv(SocketArray[i], RecvBuff, sizeof(RecvBuff)-1, 0);

                                        if(RecvBytes > 0) // Some data received.
                                        {

                                                printf("Message Send.\n");
                                                printf("Message was: %s\n",RecvBuff);

                                                for(k=0;k<=index-1;k++)
                                                {
                                                        if(FD_ISSET(SocketArray[k],&BackUpfdwrite))
                                                        {
                                                                if(SocketArray[k] != ListeningSocket && SocketArray[k] != SocketArray[i])
                                                                {
                                                                        memset(SentBuff,0,sizeof(SentBuff));
                                                                        strcpy(SentBuff,RecvBuff);
                                                                        SentBytes = send(SocketArray[k],SentBuff,sizeof(SentBuff),0);

                                                                }

                                                        }
                                                }

                                        }

                                }

                        }
                }
                SleepEx(10, FALSE);

        }// Main loop ends here


}

---

Client :

Code:

---

#include <WinSock2.h>
#include <stdio.h>
#include <time.h>



main()
{

        SOCKET                        ConnectSocket;
        SOCKET                        SocketArray[20];

        SOCKADDR_IN                ServerAddr;

        WSADATA                        wsaData;

        FD_SET                        fdwrite;
        FD_SET                        fdread;
        FD_SET                        BackUpfdread;
        FD_SET                        BackUpfdwrite;

        char                        server_address[20] = "192.168.1.65";
        char                        SentBuff[500];
        char                        RecvBuff[500];

        const unsigned short PORT = 4444;

        int                                maxDescriptor;
        int                                index = 0;
        int                                SelectResults;
        int                                i;
        int                                RecvBytes;
        int                                SentBytes;

        BOOL                        bOpt = TRUE;

        struct timeval        timeout;


        // Initialize Winsock 2.2
        WSAStartup(MAKEWORD(2,2),&wsaData);

        // Initialize ServerAddr
        ServerAddr.sin_family = AF_INET;
        ServerAddr.sin_addr.s_addr = inet_addr(server_address);
        ServerAddr.sin_port = htons(PORT);

        // Create a new socket to make a client connection.
        ConnectSocket = socket(AF_INET,SOCK_STREAM,IPPROTO_TCP);

        setsockopt(ConnectSocket, IPPROTO_TCP, TCP_NODELAY,(char*)&bOpt,sizeof(BOOL));

        // Clear the fd sets
        FD_ZERO(&fdread);
        FD_ZERO(&BackUpfdread);
        FD_ZERO(&fdwrite);
        FD_ZERO(&BackUpfdwrite);

        // Asign ConnectSocket into fdread and fdwrite.
        FD_SET(ConnectSocket,&fdread);
        FD_SET(ConnectSocket,&fdwrite);

        // Set timer
        timeout.tv_sec = 0;
        timeout.tv_usec = 0;

        maxDescriptor = ConnectSocket;
        SocketArray[index] = ConnectSocket;
        index++;

        // Make a connection to the server with socket s.
        if(connect(ConnectSocket, (SOCKADDR *) &ServerAddr, sizeof(ServerAddr)) == SOCKET_ERROR)
        {
                printf("Couldn't connect to the server\n");
        }


        // Main loop starts here
        for(; ;)
        {
                BackUpfdread = fdread;
                BackUpfdwrite = fdwrite;

                memset(SentBuff, 0, sizeof(SentBuff));
                printf("Write: ");
                gets_s(SentBuff, sizeof(SentBuff));

               

                SelectResults = select(maxDescriptor+1,&BackUpfdread,&BackUpfdwrite,NULL,&timeout);

                for(i=0;i<=index-1;i++)
                {
                        // Something to read from server.
                        if(FD_ISSET(SocketArray[i],&BackUpfdread) && SocketArray[i] == ConnectSocket)
                        {
                                RecvBytes = recv(SocketArray[i], RecvBuff, sizeof(RecvBuff), 0);

                                if(RecvBytes > 0)
                                {
                                        printf("%s\n",RecvBuff); 
                                        // Cleaning the Receive Buffer
                                        memset(RecvBuff,0,sizeof(RecvBuff));
                                }

                        }

                        // Something to write.
                        if(FD_ISSET(SocketArray[i],&BackUpfdwrite) && SocketArray[i] == ConnectSocket)
                        {
                                SentBytes = send(SocketArray[i], SentBuff,sizeof(SentBuff),0);
                                // Cleaning the Sent Buffer
                                memset(SentBuff,0,sizeof(SentBuff));
                               
                        }


                }

                SleepEx(10, FALSE);
               
        } // Main menu ends here

}

---

The problem is the delay of messages' transfer, to clients. Can somebody find where the mistake is with my app ?

Any suggestion is welcomed.

Thanks.

On the server side, as soon as you receive data from a client, you should call send() to respond. You do not need to check on readyness to write (i.e., you do not need to wait until FD_ISSET for writing). The only time you need to check on readyness to write is if you call send() and send() returns an indication that the socket is not ready to write.

In addition, your code assumes that all data (including a terminating NULL) will be received in a single call to recv(), and further assumes that all data will be sent in a single call to send(). Both are bad assumptions. TCP is a stream-based protocol, not a message-based protocol, so when using TCP, you must implement some sort of serialization when sending and receiving bytes.

Mike

First of all thank you for your answer.

I ve change the piece of server's code that sends the message to the clients removing the

if(FD_ISSET(SocketArray[k],&BackUpfdwrite))

now the server became like this :

Code:

---

}else{ // That means that the socket is not from a new conection and has something sent.

                                        memset(RecvBuff,0,sizeof(RecvBuff));
                                        RecvBytes = recv(SocketArray[i], RecvBuff, sizeof(RecvBuff)-1, 0);

                                        if(RecvBytes > 0) // Some data received.
                                        {

                                                printf("Message Sent.\n");
                                                printf("Message was: %s\n",RecvBuff);

                                                for(k=0;k<=index-1;k++)
                                                {
                                                       
                                                        if(SocketArray[k] != ListeningSocket && SocketArray[k] != SocketArray[i])
                                                        {
                                                                memset(SentBuff,0,sizeof(SentBuff));
                                                                strcpy(SentBuff,RecvBuff);
                                                                SentBytes = send(SocketArray[k],SentBuff,sizeof(SentBuff),0);

                                                                if(SentBytes > 0)
                                                                {
                                                                        printf("Message Forwarded\n");
                                                                }else{
                                                                        printf("Message forward error\n");
                                                                }

                                                        }

                                                       
                                                }

                                        }

---

the problem still exists with the delay of message transfer and this means that there is something else that make messages not be transferred on time.

How you find client's code ? Does it need select and fd_isset too ? or i have to code it in other way?

If i connect to the server two telnet clients seems to work nice and that's why i m thinking that maybe the problem is in client.

Also can you make more clear your words in the second paragraph cuz didn't understood them very well.

Quote:

---

Originally Posted by netpumber

How you find client's code ? Does it need select and fd_isset too ? or i have to code it in other way?

If i connect to the server two telnet clients seems to work nice and that's why i m thinking that maybe the problem is in client.

---

In almost all situations, client or server, you do not need to check for readiness to write before calling send(). In other words, you do not need to wait until FD_ISSET before writing. The only time you need to check on readiness to write is if you call send() and send() returns an indication that the socket is not ready to write. If that happens, then you must wait before calling send() again until FD_ISSET.

So, in general, the algorithm is:
1 - call send()
2 - did send() complete correctly? If so, then you're done; call send or recv or whatever for new data.
3 - If send() did not complete correctly, call select on a write_fd_set and then call send() once it returns with FD_ISSET

Look here for a basic client program, which uses blocking sockets (like you do): "Winsock Programmer’s FAQ - Basic Winsock Examples: Basic Blocking Client " at http://tangentsoft.net/wskfaq/exampl...ic-client.html

Other examples from the above site start here: http://tangentsoft.net/wskfaq/exampl...ics/index.html

Quote:

---

Originally Posted by netpumber

Also can you make more clear your words in the second paragraph cuz didn't understood them very well.

---

"TCP is a stream-based protocol" means that the data on the wire is a stream of individual bytes: there is no notion of packets. If you call send() quickly, three times in a row, with 50 then 100 then 150 bytes, the result will be a stream of 300 bytes on the wire. The recipient must somehow decode these into the original three calls to send() (if that's the intention). Likewise, on the receiving end, one call to recv() might result in receipt of bytes from multiple calls to send() and indeed it's possible that the recipient receives only part of the bytes from a single call to send().

For that reason, it's up to you, as the application programmer, to institute some sort of application-level protocol in order to serialize and de-serialize the data being sent.

One protocol is for the server always to terminate sent bytes with a NULL when calling send(). It's wrong, however, for the client to assume that a single call to recv() will always contain the terminating NULL that was sent from the server; rather, the client must actively look for it, and continue to call recv() until the terminating NULL is found. It also cannot disregard bytes recieved after the terminating NULL, since those bytes might contain the beginning of a new NULL-terminated message from the server.

Mike

Incidentally, I just realized that your client is using a combination of blocking sockets and select(). That makes no sense, and as you have realized, your call to select() must include a timeout or else it might never return.

Use of select() makes sense only when using non-blocking sockets.

You have set the timeout to zero, which means that you are constantly polling for activity, without any pause. That's why you also needed the SleepEx call for 10 msec, since otherwise you would be using 100% CPU. Both are bad (meaning both of polling and SleepEx to avoid 100% CPU are bad).

See better designs at the Winsock FAQ site that I linked to.

Mike