Winsock Server using WinAPI Thread reset first connection

Question

I have written a Server using WINAPI with CreateThread() Method. The first connection request always dies. All the following requests/threads are working as expected. I have no idea why so i hope someone could tell me. Here is a working example illustrating the issue.

DWORD WINAPI process_thread(LPVOID lpParam) {

    SOCKET current_client = (SOCKET)lpParam;
    char buf[1024];
    int res;

    while(1) {

        res = recv(current_client, buf, strlen(buf), 0);            
        if(res>0) {
            buf[res] = '\0';
            send(current_client, buf, strlen(buf), 0);
        }

    }
}

int main() {

    SOCKET sock;
    DWORD thread;    
    WSADATA wsaData;    
    SOCKADDR_IN server;

    WSAStartup(0x102,&wsaData);

    server.sin_family=AF_INET;
    server.sin_addr.s_addr=INADDR_ANY;
    server.sin_port=htons(123);

    sock=socket(AF_INET,SOCK_STREAM,0);    
    bind(sock,(SOCKADDR*)&server,sizeof(server));    
    listen(sock,5);

    SOCKET client;    
    SOCKADDR_IN from;
    int fromlen = sizeof(from);

    while(1) {

        client = accept(sock,(struct SOCKADDR*)&from,&fromlen);       
        CreateThread(NULL, 0,process_thread,(LPVOID)client, 0, &thread);

    }

    closesocket(sock);
    WSACleanup();

    return 0;

}

regarding: `server.sin_port=htons(123);` the first 1024 port numbers are 'reserved' for specific purposes. Suggest using a port number greater than 1024 — user3629249, Dec 15 '18 at 19:21

Remy Lebeau · Answer 1 · 2018-12-15T01:06:45.300

You are misusing strlen() inside your thread code.

When calling recv(), you need to specify the full size of your buffer. strlen() is not the right way to get that value. Use sizeof() instead.

Then, when recv() exits, its return value tells you exactly know how many bytes in the buffer are valid. Again, strlen() is not the right way to get that value.

Also, you don't need to null-terminate the buffer just to pass it to send(). Since you are told how many bytes are in the buffer, just send that many bytes.

Also, your threads are not terminating, or closing their sockets, when clients disconnect from the server.

Also, your main() is leaking thread handles, and not doing any kind of error handling at all.

Try something more like this instead:

bool sendAll(SOCKET sock, void *buf, int buflen)
{
    char *ptr = (char*) buf;
    int sent;

    while (buflen > 0) {
        sent = send(sock, ptr, buflen, 0);
        if (sent == SOCKET_ERROR) {
            return false;
        }
        ptr += sent;
        buflen -= sent;
    }

    return true;
}

DWORD WINAPI process_thread(LPVOID lpParam) {

    SOCKET client = (SOCKET) lpParam;
    char buf[1024], *ptr;
    int recvd;

    do {
        recvd = recv(client, buf, sizeof(buf), 0);            
        if (recvd <= 0) {
            break;
        }
        if (!sendAll(client, buf, recvd)) {
            break;
        }
    }
    while (true);

    closesocket(client);
    return 0;
}

int main() {

    WSADATA wsaData;
    SOCKET server, client;
    SOCKADDR_IN serveraddr;
    SOCKADDR_IN clientaddr;
    int res, clientaddrlen;
    HANDLE hThread;
    DWORD threadID;

    res = WSAStartup(MAKEWORD(2, 1), &wsaData);
    if (res != 0) {
        return 1;
    }

    ZeroMemory(&serveraddr, sizeof(serveraddr));
    serveraddr.sin_family = AF_INET;
    serveraddr.sin_addr.s_addr = INADDR_ANY;
    serveraddr.sin_port = htons(123);

    server = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    if (server == INVALID_SOCKET) {
        WSACleanup();
        return 1;
    }

    res = bind(server, (SOCKADDR*) &serveraddr, sizeof(serveraddr));
    if (res == SOCKET_ERROR) {
        closesocket(server);
        WSACleanup();
        return 1;
    }

    res = listen(server, 5);
    if (res == SOCKET_ERROR) {
        closesocket(server);
        WSACleanup();
        return 1;
    }

    do {
        clientaddrlen = sizeof(clientaddr);

        client = accept(server, (SOCKADDR*) &clientaddr, &clientaddrlen);
        if (client == INVALID_SOCKET) {
            closesocket(server);
            WSACleanup();
            return 1;
        }

        hThread = CreateThread(NULL, 0, process_thread, (LPVOID) client, 0, &threadID);
        if (hThread)
            CloseHandle(hThread);
        else
            closesocket(client);
    }
    while (true);

    closesocket(server);
    WSACleanup();

    return 0;
}

Winsock Server using WinAPI Thread reset first connection

1 Answers1