Thread: TCP Server

Hello, i have written a simple TCP server. But when i tried make it multithreaded i screw up something. Can someone help me to find out what's wrong with it?

Code:

#include <cstdlib>
#include <iostream>
#include <winsock2.h>
#include <ws2tcpip.h>

//Zmienne stale
#define DEFAULT_BUFLEN 512
#define MAX_CLIENTS 100

using namespace std;

//Deklaracja funkcji
DWORD WINAPI ServeClient(LPVOID lpParam);

//Zmienne globalne
typedef struct TThreadInfo {
    int ID;
    int cos1;
} TTHREADINFO, *PTTHREADINFO;

char *D_PORT = "6698";
bool ShutDown = 0;
HANDLE ClientThread [MAX_CLIENTS];
HANDLE ListenerHandle;
DWORD ClientThreadID [MAX_CLIENTS];
WSADATA wsaData;
SOCKET ListenSocket = INVALID_SOCKET;
SOCKET ClientSocket = INVALID_SOCKET;

struct addrinfo *result = NULL;
struct addrinfo *ptr = NULL;
struct addrinfo hints;

//Rest
bool StartWSA(void)
{
     if (WSAStartup(MAKEWORD(2,2), &wsaData) != 0) return 0; else return 1;
}

bool InitSock(void)
{
     ZeroMemory(&hints, sizeof (hints));
     hints.ai_family = AF_INET;
     hints.ai_socktype = SOCK_STREAM;
     hints.ai_protocol = IPPROTO_TCP;
     hints.ai_flags = AI_PASSIVE;
     // Resolve the local address and port to be used by the server
     if ((getaddrinfo(NULL, D_PORT, &hints, &result)) != 0) return 0; else return 1;   
}

bool CreateSock(void)
{
     // Create a SOCKET for the server to listen for client connections
     ListenSocket = socket(result->ai_family, result->ai_socktype, result->ai_protocol);
     if (ListenSocket == INVALID_SOCKET) return 0; else return 1;
     //
    // for(int a=0;a<MAX_CLIENTS;a++) ClientSocket[a] = INVALID_SOCKET;
}

bool BindSock(void)
{
     // Setup the TCP listening socket
     int _result;
     _result = bind( ListenSocket, result->ai_addr, (int)result->ai_addrlen);
     if (_result == SOCKET_ERROR) return 0; else return 1;
}

bool Listener(void)
{
     if ( listen( ListenSocket, SOMAXCONN ) == SOCKET_ERROR ) return 0; else return 1;
}

DWORD WINAPI ServeClient(LPVOID lpParam)
{      
      printf("Serving client..");
      bool CloseConnection = 0;
      char recvbuf[DEFAULT_BUFLEN];
      int sResult, cResult;
      int recvbuflen = DEFAULT_BUFLEN;
      SOCKET current_sock;
      current_sock = (SOCKET)lpParam;
      do {
      cResult = recv(current_sock, recvbuf, recvbuflen, 0);
      if (cResult > 0) {
        printf("Bytes received: %d\n", cResult);

        // Echo the buffer back to the sender
        sResult = send(current_sock, recvbuf, cResult, 0);
        if (sResult == SOCKET_ERROR) {
            printf("send failed: %d\n", WSAGetLastError());
            closesocket(current_sock);
            WSACleanup();
            CloseConnection = 1;
            return 1;
        }
        printf("Bytes sent: %d\n", sResult);
      } else {
        printf("Connection error..");
        closesocket(current_sock);
        WSACleanup();
        CloseConnection = 1;
        return 1;
    }
  } while (!CloseConnection); 
}

int main(int argc, char *argv[])
{
    if (argv[1] != 0) D_PORT = argv[1];
    if (!StartWSA()) 
    {
       printf("- WSA Startup error\n"); 
       system("PAUSE");
       return 1; 
    }
    printf("* WSA started\n");
    
    if (!InitSock()) 
    {
        printf("- GetAddrInfo failed\n");
        WSACleanup();
        system("PAUSE");
        return 1;
    }
    printf("* Socket initialized\n");

    if (!CreateSock())
    {
        printf("- Error at socket\n");
        freeaddrinfo(result);
        WSACleanup();
        system("PAUSE");
        return 1;
    }
    printf("* Socket created\n");
    
    if (!BindSock())
    {
        printf("- Bind failed\n");
        freeaddrinfo(result);
        closesocket(ListenSocket);
        WSACleanup();
        system("PAUSE");
        return 1;
    }
    printf("* Binging socket success\n");
    
    if (!Listener())
    {
        printf("- Listen failed\n");
        closesocket(ListenSocket);
        WSACleanup();
        system("PAUSE");
        return 1;
    }
    printf("* Listener started\n");
    printf("* Server is running :\n");
    printf("  - IP : 127.0.0.1\n");
    printf("  - PORT : %s\n", D_PORT);

do
  {
      ClientSocket = accept(ListenSocket, NULL, NULL);
      if (ClientSocket == INVALID_SOCKET) {
        printf("Client connection failed\n");
        // closesocket(ListenSocket);
        //WSACleanup();        
        }  
      printf("Client connected");
      
      DWORD thread;
      CreateThread(
        NULL,                        // no security attributes 
        0,                           // use default stack size  
        ServeClient,                  // thread function 
        (LPVOID)ClientSocket,                // argument to thread function 
        0,                           // use default creation flags 
        &thread);
      ClientSocket = INVALID_SOCKET;
  }
  while (true);

    system("PAUSE");
    closesocket(ListenSocket);
    WSACleanup();
    return EXIT_SUCCESS;
}

The thing happen is that im getting in infinite loop here :

Code:

do
  {
      ClientSocket = accept(ListenSocket, NULL, NULL);
      if (ClientSocket == INVALID_SOCKET) {
        printf("Client connection failed\n");
        // closesocket(ListenSocket);
        //WSACleanup();        
        }  
      printf("Client connected");
      
      DWORD thread;
      CreateThread(
        NULL,                        // no security attributes 
        0,                           // use default stack size  
        ServeClient,                  // thread function 
        (LPVOID)ClientSocket,                // argument to thread function 
        0,                           // use default creation flags 
        &thread);
      ClientSocket = INVALID_SOCKET;
  }
  while (true);

Why it's not waiting for next connection and getting old one?

When the client closes the connection recv will return 0. You should not be treating that as an error condition and subsequently cleaning up Winsock via WSACleanup.

instead of calling accept like this:

Code:

ClientSocket = accept(ListenSocket, NULL, NULL);

try passing it a sockaddr_in struct and its length for it to fill with the client info.

accept() winsock reference

Thanks it works this way

Code:

struct sockaddr_in clientinfo;
int cilen = sizeof(struct sockaddr);
ClientSocket = accept(ListenSocket, (LPSOCKADDR)&clientinfo, &cilen);
printf(inet_ntoa(clientinfo.sin_addr));

One more question, sometimes when i try send two packets, server interprets it as one packet like :

1 - I send byte 0x01
2 - I send buff with text (0x5A3D7C....)

The server interprets it as one : 015A3D7C..
So i do something wrong or it is how it works? In delphi i hadn't that problem, everytime i got separated packets.