Winsock2, client-server communication - send/recv in turns

Question

I want to write a client/server application, in which client and server can exchange messages.

Client site communication:

send
recv
send 
recv

Server site communication:

recv
send
recv
send

However, I have a problem, because only one message is send/recv. After that, the socket is closed and further messages are not being sent. Whats wrong and how to fix this? Thank you.

Server code:

#undef UNICODE

#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <stdlib.h>
#include <stdio.h>
#pragma comment (lib, "Ws2_32.lib")
#define DEFAULT_PORT "27501"
#define SIZE 1024

int SendAllToClient(SOCKET ClientSocket, char *buffer)
{
    int iSendResult;


    int total = 0, len = 1024;
    int bytesleft = 1024;

    while( total < len )
    {
        iSendResult = send( ClientSocket, buffer, 1024, NULL);

        if (iSendResult == SOCKET_ERROR)
        {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ClientSocket);
            WSACleanup();
            return 1;
        }
        total += iSendResult;
        bytesleft -= iSendResult;
    }
    printf("Bytes sent: %d\n", iSendResult);

    return total<len?- 1: 1;
}

char *ReadFromClient(SOCKET ConnectSocket)
{
    int iResult;
    char *buffer = new char[1024];
    memset(buffer, 0, 1024);

    do
    {
        iResult = recv(ConnectSocket, buffer, 1024, 0);
        if ( iResult > 0 )
            printf("Bytes received: %d\n", iResult);
        else if ( iResult == 0 )
            printf("Connection closed\n");

        else
            printf("recv failed with error: %d\n", WSAGetLastError());

    }
    while( iResult > 0 );
    return buffer;
}

int main(int argc , char *argv[])
{
    WSADATA wsaData;
    char *buffer;

    SOCKET ListenSocket = INVALID_SOCKET;
    SOCKET ClientSocket = INVALID_SOCKET;

    struct addrinfo *result = NULL;
    struct addrinfo hints;

    int sessionID;
    int iResult;

   // Datagram d1,d2,d3;

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0)
    {
        printf("WSAStartup failed with error: %d\n", iResult);
        return 1;
    }

    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 server address and port
    iResult = getaddrinfo(NULL, DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 )
    {
        printf("getaddrinfo failed with error: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    // Create a SOCKET for connecting to server
    ListenSocket = socket(result->ai_family, result->ai_socktype, result->ai_protocol);
    if (ListenSocket == INVALID_SOCKET)
    {
        printf("socket failed with error: %d\n", WSAGetLastError());
        freeaddrinfo(result);
        WSACleanup();
        return 1;
    }

    // Setup the TCP listening socket
    iResult = bind( ListenSocket, result->ai_addr, (int)result->ai_addrlen);
    if (iResult == SOCKET_ERROR)
    {
        printf("bind failed with error: %d\n", WSAGetLastError());
        freeaddrinfo(result);
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    freeaddrinfo(result);

    iResult = listen(ListenSocket, SOMAXCONN);
    if (iResult == SOCKET_ERROR)
    {
        printf("listen failed with error: %d\n", WSAGetLastError());
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    printf("Server is listening on localhost:%s ...\n", DEFAULT_PORT);

    // Accept a client socket
    SOCKADDR_IN addr;
    int addrlen = sizeof(addr);

    ClientSocket = accept(ListenSocket, (SOCKADDR*)&addr, &addrlen);
    if (ClientSocket == INVALID_SOCKET)
    {
        printf("accept failed with error: %d\n", WSAGetLastError());
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    char *ip = inet_ntoa(addr.sin_addr);
    int port = addr.sin_port;
    printf("\nClient %s:%d connected to server\n", ip, port);

    // No longer need server socket
    closesocket(ListenSocket);

    // shutdown the connection since we're done
    iResult = shutdown(ClientSocket, SD_SEND);
    if (iResult == SOCKET_ERROR)
    {
        printf("shutdown failed with error: %d\n", WSAGetLastError());
        closesocket(ClientSocket);
        WSACleanup();
        return 1;
    }

    //read
    buffer = ReadFromClient(ClientSocket);
    printf("FROM CLIENT: %s\n", buffer);
    //send
    memset(buffer, 0, 1024);
    memcpy(buffer, "Hi client, how are you?", strlen("Hi client, how are you?"));
    SendAllToClient(ClientSocket, buffer);
    //read
    //send

    // cleanup
    closesocket(ClientSocket);
    WSACleanup();

    return 0;
}

Client code:

#undef UNICODE

#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <stdlib.h>
#include <stdio.h>
#pragma comment (lib, "Ws2_32.lib")
#define DEFAULT_PORT "27501"
#define SIZE 1024

int SendAllToServer(SOCKET ServerSocket, char *buffer)
{
    int iSendResult;

    int total = 0, len = 1024;
    int bytesleft = 1024;

    while( total < len )
    {
        iSendResult = send( ServerSocket, buffer, 1024, NULL);

        if (iSendResult == SOCKET_ERROR)
        {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ServerSocket);
            WSACleanup();
            return 1;
        }
        total += iSendResult;
        bytesleft -= iSendResult;
    }
    printf("Bytes sent: %d\n", iSendResult);

    return total<len?- 1: 1;
}

char *ReadFromServer(SOCKET ConnectSocket)
{
    int iResult;
    char *buffer = new char[1024];
    memset(buffer, 0, 1024);

    do {
        iResult = recv(ConnectSocket, buffer, 1024, 0);
        if ( iResult > 0 )
            printf("Bytes received: %d\n", iResult);
        else if ( iResult == 0 )
            printf("Connection closed\n");

        else
            printf("recv failed with error: %d\n", WSAGetLastError());

    } while( iResult > 0 );
    return buffer;
}

int main(int argc , char *argv[])
{
    WSADATA wsaData;
    SOCKET ConnectSocket = INVALID_SOCKET;
    struct addrinfo *result = NULL,
                    *ptr = NULL,
                    hints;
    char* buffer;
    int sessionID;
    int iResult;
   // Datagram d1,d2;

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0) {
        printf("WSAStartup failed with error: %d\n", iResult);
        return 1;
    }

    ZeroMemory( &hints, sizeof(hints) );
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    // Resolve the server address and port
    iResult = getaddrinfo("127.0.0.1", DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 ) {
        printf("getaddrinfo failed with error: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    // Attempt to connect to an address until one succeeds
    for(ptr=result; ptr != NULL ;ptr=ptr->ai_next) {

        // Create a SOCKET for connecting to server
        ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype,
            ptr->ai_protocol);
        if (ConnectSocket == INVALID_SOCKET) {
            printf("socket failed with error: %d\n", WSAGetLastError());
            WSACleanup();
            return 1;
        }

        // Connect to server.
        iResult = connect( ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
        if (iResult == SOCKET_ERROR) {
            closesocket(ConnectSocket);
            ConnectSocket = INVALID_SOCKET;
            continue;
        }
        break;
    }

    freeaddrinfo(result);

    if (ConnectSocket == INVALID_SOCKET) {
        printf("Unable to connect to server!\n");
        WSACleanup();
        return 1;
    }

    //send
    buffer = new char[1024];
    memset(buffer, 0, 1024);
    memcpy(buffer, "Hi server", strlen("Hi server"));
    SendAllToServer(ConnectSocket, buffer);
    //read
    memset(buffer, 0, 1024);
    buffer = ReadFromServer(ConnectSocket);
    printf("FROM SERVER: %s\n", buffer);
    //send
    //read

    // cleanup
    closesocket(ConnectSocket);
    WSACleanup();

    return 0;
}

EDIT

Server

#include "data_types.h"
#undef UNICODE

#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <stdlib.h>
#include <stdio.h>
#include <string>
#include <iostream>
#pragma comment (lib, "Ws2_32.lib")
#define DEFAULT_PORT "27015"
#define SIZE 1024

int SendAllToClient(SOCKET ClientSocket)
{
    char *buffer = new char[SIZE];
    int iSendResult;
    memset(buffer, 0, SIZE);

    int total = 0, len = SIZE;
    int bytesleft = SIZE;

    while( total < len )
    {
        iSendResult = send( ClientSocket, buffer + total, bytesleft, NULL);

        if (iSendResult == SOCKET_ERROR)
        {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ClientSocket);
            WSACleanup();
            return 1;
        }
        total += iSendResult;
        bytesleft -= iSendResult;
    }
    printf("Bytes sent: %d\n", iSendResult);
    delete buffer;

    return total<len?- 1: 1;
}

char* ReadFromClient(SOCKET ClientSocket)
{
    std::string data = "";
    char *all = NULL;
    char buffer[1];
    int total = 0, len = SIZE;
    int bytesleft = SIZE;
    int iResult;
    memset(buffer, 0, 1);

    while(total < len)
    {
        if ((iResult = recv(ClientSocket, buffer, 1, 0)) == 0)
        {
            if (errno != 0)
            {
                // cleanup
                closesocket(ClientSocket);
                WSACleanup();
                exit(1);
            }
        }
        data = data + std::string(buffer);
        total += iResult;
        bytesleft -= iResult;
        memset(buffer, 0, 1);
    }
    all = new char[data.length() + 1];
    strcpy(all, data.c_str());

    return all;
}

int main(int argc , char *argv[])
{
    WSADATA wsaData;
    char *buffer;

    SOCKET ListenSocket = INVALID_SOCKET;
    SOCKET ClientSocket = INVALID_SOCKET;

    struct addrinfo *result = NULL;
    struct addrinfo hints;

    int sessionID;
    int iResult;

   // Datagram d1,d2,d3;

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0)
    {
        printf("WSAStartup failed with error: %d\n", iResult);
        return 1;
    }

    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 server address and port
    iResult = getaddrinfo(NULL, DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 )
    {
        printf("getaddrinfo failed with error: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    // Create a SOCKET for connecting to server
    ListenSocket = socket(result->ai_family, result->ai_socktype, result->ai_protocol);
    if (ListenSocket == INVALID_SOCKET)
    {
        printf("socket failed with error: %d\n", WSAGetLastError());
        freeaddrinfo(result);
        WSACleanup();
        return 1;
    }

    // Setup the TCP listening socket
    iResult = bind( ListenSocket, result->ai_addr, (int)result->ai_addrlen);
    if (iResult == SOCKET_ERROR)
    {
        printf("bind failed with error: %d\n", WSAGetLastError());
        freeaddrinfo(result);
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    freeaddrinfo(result);

    iResult = listen(ListenSocket, SOMAXCONN);
    if (iResult == SOCKET_ERROR)
    {
        printf("listen failed with error: %d\n", WSAGetLastError());
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    printf("Server is listening on localhost:%s ...\n", DEFAULT_PORT);

    // Accept a client socket
    SOCKADDR_IN addr;
    int addrlen = sizeof(addr);

    ClientSocket = accept(ListenSocket, (SOCKADDR*)&addr, &addrlen);
    if (ClientSocket == INVALID_SOCKET)
    {
        printf("accept failed with error: %d\n", WSAGetLastError());
        closesocket(ListenSocket);
        WSACleanup();
        return 1;
    }

    char *ip = inet_ntoa(addr.sin_addr);
    int port = addr.sin_port;
    printf("\nClient %s:%d connected to server\n", ip, port);

    // No longer need server socket
    closesocket(ListenSocket);

    // shutdown the connection since we're done
    iResult = shutdown(ClientSocket, SD_SEND);
    if (iResult == SOCKET_ERROR)
    {
        printf("shutdown failed with error: %d\n", WSAGetLastError());
        closesocket(ClientSocket);
        WSACleanup();
        return 1;
    }

    //read
    buffer = new char[1024];
    memset(buffer, 0, 1024);
    buffer = ReadFromClient(ClientSocket);
    printf("FROM CLIENT: %s\n", buffer);

    //send
    memset(buffer, 0, 1024);
    memcpy(buffer, "Hi client, how are you?", strlen("Hi client, how are you?"));
    // Send an initial buffer
    iResult = send( ClientSocket, buffer, (int)strlen(buffer), 0 );
    if (iResult == SOCKET_ERROR) {
        wprintf(L"send failed with error: %d\n", WSAGetLastError());
        closesocket(ClientSocket);
        WSACleanup();
        return 1;
    }
    //read
    //send

    // cleanup
    closesocket(ClientSocket);
    WSACleanup();

    return 0;
}

Client

#include "data_types.h"
#undef UNICODE

#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <stdlib.h>
#include <stdio.h>
#include <string>
#include <iostream>
#pragma comment (lib, "Ws2_32.lib")
#define DEFAULT_PORT "27015"
#define SIZE 1024

int SendAllToServer(SOCKET ClientSocket)
{
    char *buffer = new char[SIZE];
    memset(buffer, 0, SIZE);

    int total = 0, len = SIZE;
    int bytesleft = SIZE, iSendResult;

    while( total < len )
    {
        iSendResult = send( ClientSocket, buffer + total, bytesleft, NULL);

        if (iSendResult == SOCKET_ERROR)
        {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ClientSocket);
            WSACleanup();
            return 1;
        }
        total += iSendResult;
        bytesleft -= iSendResult;
    }
    printf("Bytes sent: %d\n", iSendResult);
    delete buffer;

    return total<len?- 1: 1;
}

char *ReadFromServer(SOCKET ClientSocket)
{
    std::string data = "";
    char *all = NULL;
    char buffer[1];
    int total = 0, len = SIZE;
    int bytesleft = SIZE;
    int iResult;
    memset(buffer, 0, 1);

    while(total < len)
    {
        if ((iResult = recv(ClientSocket, buffer, 1, 0)) == 0)
        {
            if (errno != 0)
            {
                // cleanup
                closesocket(ClientSocket);
                WSACleanup();
                exit(1);
            }
        }
        data = data + std::string(buffer);
        total += iResult;
        bytesleft -= iResult;
        memset(buffer, 0, 1);
    }
    all = new char[data.length() + 1];
    strcpy(all, data.c_str());

    return all;
}

int main(int argc , char *argv[])
{
    WSADATA wsaData;
    SOCKET ConnectSocket = INVALID_SOCKET;
    struct addrinfo *result = NULL,
                    *ptr = NULL,
                    hints;
    char* buffer;
    int sessionID;
    int iResult;
   // Datagram d1,d2;

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0) {
        printf("WSAStartup failed with error: %d\n", iResult);
        return 1;
    }

    ZeroMemory( &hints, sizeof(hints) );
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    // Resolve the server address and port
    iResult = getaddrinfo("127.0.0.1", DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 ) {
        printf("getaddrinfo failed with error: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    // Attempt to connect to an address until one succeeds
    for(ptr=result; ptr != NULL ;ptr=ptr->ai_next) {

        // Create a SOCKET for connecting to server
        ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype,
            ptr->ai_protocol);
        if (ConnectSocket == INVALID_SOCKET) {
            printf("socket failed with error: %d\n", WSAGetLastError());
            WSACleanup();
            return 1;
        }

        // Connect to server.
        iResult = connect( ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
        if (iResult == SOCKET_ERROR) {
            closesocket(ConnectSocket);
            ConnectSocket = INVALID_SOCKET;
            continue;
        }
        break;
    }

    freeaddrinfo(result);

    if (ConnectSocket == INVALID_SOCKET) {
        printf("Unable to connect to server!\n");
        WSACleanup();
        return 1;
    }

    //send
    buffer = new char[1024];
    memset(buffer, 0, 1024);
    memcpy(buffer, "Hi server, how are you?", strlen("Hi server, how are you?"));
    // Send an initial buffer
    iResult = send( ConnectSocket, buffer, (int)strlen(buffer), 0 );
    if (iResult == SOCKET_ERROR) {
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }
    //read
    memset(buffer, 0, 1024);
    // Receive until the peer closes the connection
    do {

        iResult = recv(ConnectSocket, buffer, 1024, 0);
        if ( iResult > 0 )
            wprintf(L"Bytes received: %d\n", iResult);
        else if ( iResult == 0 )
            wprintf(L"Connection closed\n");
        else
            wprintf(L"recv failed with error: %d\n", WSAGetLastError());

    } while( iResult > 0 );
    printf("FROM SERVER: %s\n", buffer);
    //send
    //read

    // cleanup
    closesocket(ConnectSocket);
    WSACleanup();

    return 0;
}

Answer 1

First issue:

    iSendResult = send( ClientSocket, buffer, 1024, NULL);

That 1024 should be bytesleft. If you've already read 512 bytes, you don't want to read more than another 512.

That buffer should be buffer + total. If you've already sent 512 bytes, you don't want to send the same 512 bytes again, you want to send the other 512 bytes.

Second issue:

Your ReadFromServer function is just completely broken and doesn't follow the right logic. It only returns on a fatal error and doesn't even try to read exactly 1,024 bytes. It just reads up to 1,024 bytes and then, no matter how many bytes it actually read, tries to read 1,024 bytes again -- and at the same address overwriting whatever part of the message it already read!

It should work just like the send function, first trying to receive 1,024 bytes and looping if it receives less than that until it has either received exactly 1,024 bytes or gotten a fatal error.

Third issue:

    buffer = ReadFromServer(ConnectSocket);
    printf("FROM SERVER: %s\n", buffer);

Don't do this. Suppose the server is malicious and sent you 1,024 bytes that are not a legal C-style string. Passing that to printf through %s could cause the client to crash or misbehave. Always treat data received from the network as untrusted and potentially hostile. Though your code will work without fixing this, one day doing this will bite you in some awful way and it's not a good habit to get into.

You should also delete[] the buffer when you're done with it. But why would you return a raw pointer anyway? You could return a std::string, a std::vector, or many other much better mechanisms that avoid the risk of leaks and make copies safe.

Answer 2

In fact you are closing the listen socket, so once there is an accepted connection it won't accept any more:

ClientSocket = accept(ListenSocket, (SOCKADDR*)&addr, &addrlen);
if (ClientSocket == INVALID_SOCKET)
{
    printf("accept failed with error: %d\n", WSAGetLastError());
    closesocket(ListenSocket);
    WSACleanup();
    return 1;
}

char *ip = inet_ntoa(addr.sin_addr);
int port = addr.sin_port;
printf("\nClient %s:%d connected to server\n", ip, port);

// No longer need server socket
closesocket(ListenSocket);

Closing the listener prevents further connections. Also, you are closing the client socket before handling it:

// shutdown the connection since we're done
iResult = shutdown(ClientSocket, SD_SEND);
if (iResult == SOCKET_ERROR)
{
    printf("shutdown failed with error: %d\n", WSAGetLastError());
    closesocket(ClientSocket);
    WSACleanup();
    return 1;
}

//read
buffer = new char[1024];
memset(buffer, 0, 1024);
buffer = ReadFromClient(ClientSocket);
printf("FROM CLIENT: %s\n", buffer);

//send
memset(buffer, 0, 1024);
memcpy(buffer, "Hi client, how are you?", strlen("Hi client, how are you?"));
// Send an initial buffer
iResult = send( ClientSocket, buffer, (int)strlen(buffer), 0 );

If you want to send and receive several times, you should read/write in a loop. You are doing that only once (and doing it incorrectly): so only one message as you describe in your question.