1

I have the following code and I`m not sure why wont it work properly .

Its a multithreaded TCP server that loops accept() calls and triggers a designated

thread each time .

The problem is that the accept at times wont block , thus resulting

the program to open a new thread when there's theoretically no connection .

That's the loop -

    for (dwI = 0;; dwI++)                       //Accept MAX_CLIENTS connections
{
    if(MAX_CLIENTS == dwI)
    {
        dwI=0;
        continue;
    }//if

    if(clients[dwI].bIsInUse)
    {
        continue;
    }//if

    ZeroMemory(&from,sizeof(from));

    if(!AcceptConnection(&ServerSock,&from,&ClientSock))
    {
        PRINT_LE("AcceptConnection",ERROR_ACCEPT_SERVER_CONNECTION);
        closesocket(ServerSock);
        WSACleanup();
        return EXIT_FAILURE;
    }//if

    clients[dwI].ClientSock = ClientSock;

    if(! (clients[dwI].hThread = CreateThread(
            NULL,               //Not inheritable
            0,                  //Default stack size
            ThreadedAcceptTCP,       //ThreadedAccept - function
            &clients[dwI],//Pass pointer to the socket
            0,                  //Start immidiately
            &clients[dwI].dwThreadId                //Save thread id
            )))
    {
        PRINT_GLE("CreateThread");
        closesocket(ServerSock);
        WSACleanup();
        return EXIT_FAILURE;
    }//if

    #ifdef PRINT_STATUS                     //Print status if macro is defined
        printf("Server responce message has been sent.\n");
    #endif
}//for

With my own wrappers to each function .

AcceptConnection has the code below -

    SOCKET ClientSocket = INVALID_SOCKET;       //Client socket
INT sockaddrSize = sizeof(*pSockAddr);

ClientSocket = accept(          //Create client accepting socket
                *pSock,         //Listen-ed socket
                pSockAddr,      
                &sockaddrSize   
                );

if (INVALID_SOCKET == ClientSocket)     //Check for errors - if any - cleanup and return failure
{
    PRINT_WSAGLE("socket");
    return FAILURE;
}//if

*pClientSock = ClientSocket;        //Pass socket

return SUCCESS;

The problem occurs when I connect to the server through my browser ,

for example ,

after the first thread is done (I`ve made this sure by temporarily sleeping the main thread for 5 seconds)

it cleans everything and closes the client socket ,

though on the second accept call - it will return with the same

SOCKADDR information and cause an extra thread to go up ,

receiving the exact same data , sending the exact same data .

And printing 2 (and at times even more) times :

"Server responce message has been sent."

I could'nt figure out why this happens and hopefully you guys could help me out .

Thanks !

Danny Shemesh
  • 67
  • 1
  • 9

2 Answers2

0

This is a bit of a guess, but I wonder about the logic in this line of code:

if(!AcceptConnection(&ServerSock,&from,&ClientSock))

It is expecting AcceptConnection to return a non-zero value when it succeeds and a zero value when it fails. However, the function returns a constant SUCCESS when it succeeds. However, some of the Windows header files defined the constant SUCCESS to be 0. And various constants for failure are some non-zero value.

Even if you are defining SUCCESS and FAILURE in your own code, it might make sense to specifically check the return value such as:

if (FAILURE == AcceptConnection(&ServerSock,&from,&ClientSock))
Mark Wilkins
  • 40,729
  • 5
  • 57
  • 110
  • Hello Mark , Thank you for your comment ! This is indeed a bit baffling and unreadable and I`ll change it . I do , in fact , define them myself as (SUCCESS 1 ; FAILURE 0) And use those in the entire solution . Though debugging suggests this is not the problem sadly , and I have no idea what is its source . – Danny Shemesh Dec 27 '12 at 00:18
0

One possible issue: it doesn't look like you are setting clients[dwI].bIsInUse to true after assigning a new client SOCKET, which will mess up your logic in your for loop.

khorton
  • 69
  • 2