Winsock API - send and receive data simultaneously in two threads

Question

I'm trying to build a client and a server in the same program. For example, user 1 sends a packet of data to user 2, user 2 after receiving the packet sends back a different packet to user 1. The problem is, after running the program neither user receives the packets.If I build the client in a separate program from the server it works

#pragma comment(lib,"ws2_32.lib")
#include <WinSock2.h>
#include <iostream>
#include <thread>

static const int num_threads = 2;

static char buffer[8096 + 1];
char a[256] = {};
char MOTD[256];

void call_from_thread(int tid) {
//  ---------- Server code: ---------- //
if( tid == 0 ){

WSAData wsaData;
WORD DllVersion = MAKEWORD(2, 1);
if (WSAStartup(DllVersion, &wsaData) != 0){MessageBoxA(NULL, "WinSock startup failed", "Error", MB_OK | MB_ICONERROR);}

    SOCKADDR_IN addr;
    int addrlen = sizeof(addr);
    addr.sin_addr.s_addr = inet_addr("127.0.0.1");
    addr.sin_port = htons(1112);
    addr.sin_family = AF_INET;

    SOCKET sListen = socket(AF_INET, SOCK_STREAM, NULL);
    bind(sListen, (SOCKADDR*)&addr, sizeof(addr));
    listen(sListen, SOMAXCONN);

    SOCKET newConnection;
    newConnection = accept(sListen, (SOCKADDR*)&addr, &addrlen);
    if (newConnection == 0){
        std::cout << "Failed to accept the client's connection." << std::endl;
    }else{
        std::cout << "Client Connected!" << std::endl;
    }

    while (true){
        std::cin >> a;
        send(newConnection, a, sizeof(a), NULL);
    }
}else if( tid == 1 ){


//  ---------- Client code: ---------- //
    WSAData wsaData;
    WORD DllVersion = MAKEWORD(2, 1);
    if (WSAStartup(DllVersion, &wsaData) != 0){MessageBoxA(NULL, "Winsock startup failed", "Error", MB_OK | MB_ICONERROR);}

    SOCKADDR_IN addr;
    int sizeofaddr = sizeof(addr);
    addr.sin_addr.s_addr = inet_addr("127.0.0.1");
    addr.sin_port = htons(1111);
    addr.sin_family = AF_INET;

    SOCKET Connection = socket(AF_INET, SOCK_STREAM, NULL); //Set Connection socket
    if (connect(Connection, (SOCKADDR*)&addr, sizeofaddr) != 0) //If we are unable to connect...
    {
        MessageBoxA(NULL, "Failed to Connect", "Error", MB_OK | MB_ICONERROR);
        //return 0; //Failed to Connect
    }

    std::cout << "Connected!" << std::endl;

    char MOTD[256];
    while (true){
        recv(Connection, MOTD, sizeof(MOTD), NULL); //Receive Message of the Day buffer into MOTD array
        std::cout << "MOTD:" << MOTD << std::endl;
    }
}



//  ---------- Thread selection: ---------- //
int main() {
std::thread t[num_threads];

for (int i = 0; i < num_threads; ++i) {
    t[i] = std::thread(call_from_thread, i);
 }

 for (int i = 0; i < num_threads; ++i) {
     t[i].join();
 }

}

Answer 1

The server is listening on port 1112, but the client is connecting to port 1111. This means the client and server cannot possibly be connected to each other at all, whether they are in the same application or not.

I also see a number of other problems with your code, including:

• WSAStartup() should be called once at app startup, not per thread.

• Neither thread quits cleanly if any WinSock function fails.

• The client thread does not wait for the server thread to open its listening port before the client can then connect to it.

• Potential buffer overflows.

• Resource leaks.

• A lack of decent error handling.

  • On the server side, you are completely ignoring errors reported by socket(), bind(), listen(), and send(), and you are not checking the return value of accept() correctly.
  • On the client side, you are completely ignoring errors reported by socket() and recv(). And, you are not ensuring the MOTD buffer is null-terminated before printing it to std::cout.

Try something more like this:

#include <winsock2.h>
#include <windows.h>
#pragma comment(lib,"ws2_32.lib")

#include <iostream>
#include <thread>
#include <string>
#include <cstdint>
#include <stdexcept>
#include <limits>
#include <algorithm>

// The below variables are used to let the client wait for the server
// port to be opened. If you don't want to use these, especially if the
// client and server are ever run on different machines, you can omit
// these and instead just have the client call connect() in a loop
// until successful...
//
#include <mutex>
#include <condition_variable>
#include <chrono>
static std::condition_variable server_cv;
static std::mutex server_mtx;
static bool server_is_listening = false;
//

static const int num_threads = 2;
static const u_short server_port = 1111;

class winsock_error : public std::runtime_error
{
public:
    int errCode;
    std::string funcName;
    winsock_error(int errCode, const char *funcName) : std::runtime_error("WinSock error"), errCode(errCode), funcName(funcName) {}
};

void WinSockError(const char *funcName, int errCode = WSAGetLastError())
{
    throw winsock_error(errCode, funcName);
}

class connection_closed : public std::runtime_error
{
public:
    connection_closed() : std::runtime_error("Connection closed") {}
};

class socket_ptr
{
public:
    socket_ptr(SOCKET s) : m_sckt(s) {}
    socket_ptr(const socket_ptr &) = delete;
    socket_ptr(socket_ptr &&src) : m_sckt(src.m_sckt) { src.m_sckt = INVALID_SOCKET; }
    ~socket_ptr() { if (m_sckt != INVALID_SOCKET) closesocket(m_sckt); }

    socket_ptr& operator=(const socket_ptr &) = delete;
    socket_ptr& operator=(socket_ptr &&rhs) { m_sckt = rhs.m_sckt; rhs.m_sckt = INVALID_SOCKET; return *this; }

    operator SOCKET() { return m_sckt; }
    bool operator!() const { return (m_sckt == INVALID_SOCKET); }

private:
    SOCKET m_sckt;
}

template <typename T>
T LimitBufferSize(size_t size)
{
    return (T) std::min(size, (size_t) std::numeric_limits<T>::max());
}

void sendRaw(SOCKET sckt, const void *buffer, size_t buflen)
{
    const char *ptr = static_cast<const char*>(buffer);
    while (buflen > 0)
    {
        int numToSend = LimitBufferSize<int>(buflen);
        int numSent = ::send(sckt, ptr, numToSend, 0);
        if (numSent == SOCKET_ERROR)
            WinSockError("send");
        ptr += numSent;
        buflen -= numSent;
    }
}

void recvRaw(SOCKET sckt, void *buffer, size_t buflen)
{
    char *ptr = static_cast<char*>(buffer);
    while (buflen > 0)
    {
        int numToRecv = LimitBufferSize<int>(buflen);
        int numRecvd = ::recv(sckt, ptr, numToRecv, 0);
        if (numRecvd == SOCKET_ERROR)
            WinSockError("recv");
        if (numRecvd == 0)
            throw connection_closed();
        ptr += numRecvd;
        buflen -= numRecvd;
    }
}

void sendStr(SOCKET sckt, const std::string &str)
{
    uint32_t len = LimitBufferSize<uint32_t>(str.size());
    uint32_t tmp = htonl(len);
    sendRaw(sckt, &tmp, sizeof(tmp));
    if (len > 0)
        sendRaw(sckt, str.c_str(), len);
}

std::string recvStr(SOCKET sckt)
{
    std::string str;
    uint32_t len;
    recvRaw(sckt, &len, sizeof(len));
    len = ntohl(len);
    if (len > 0)
    {
        str.resize(len);
        recvRaw(sckt, &str[0], len);
    }
    return str;
}

void server_thread()
{
    std::cout << "[Server] Starting ..." << std::endl;

    try
    {
        socket_ptr sListen(::socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
        if (!sListen)
            WinSockError("socket");

        SOCKADDR_IN addr = {};
        addr.sin_family = AF_INET;
        addr.sin_addr.s_addr = ::inet_addr("127.0.0.1");
        addr.sin_port = ::htons(server_port);

        int addrlen = sizeof(addr);

        if (::bind(sListen, (SOCKADDR*)&addr, sizeof(addr)) == SOCKET_ERROR)
            WinSockError("bind");

        if (::listen(sListen, 1) == SOCKET_ERROR)
            WinSockError("listen");

        // this is optional...
        {
        std::unique_lock<std::mutex> lk(server_mtx);
        server_is_listening = true;
        }
        server_cv.notify_all();
        //

        std::cout << "[Server] Listening for Client ..." << std::endl;

        socket_ptr newConnection(::accept(sListen, (SOCKADDR*)&addr, &addrlen));
        if (!newConnection)
            WinSockError("accept");

        std::cout << "[Server] Client Connected!" << std::endl;

        try
        {
            std::string a;
            while (std::cin >> a)
                sendStr(newConnection, a);
        }
        catch (const connection_closed &)
        {
        }
        catch (const winsock_error &e)
        {
            std::cerr << "[Server] Client error: " << e.errCode << " on WinSock function: " << e.funcName << std::endl;
        }

        std::cout << "[Server] Client Disconnected!" << std::endl;
    }
    catch (const winsock_error &e)
    {
        std::cerr << "[Server] Error: " << e.errCode << " on WinSock function: " << e.funcName << std::endl;
    }
    catch (const std::exception &e)
    {
        std::cerr << "[Server] Unexpected Error! " << e.what() << std::endl;
    }

    std::cout << "[Server] Stopped!" << std::endl;
}

void client_thread()
{
    std::cout << "[Client] Starting ..." << std::endl;

    try
    {
        // this is optional, could call connect() below in a loop instead...
        std::cout << "[Client] Waiting for Server ..." << std::endl;
        {
        std::unique_lock<std::mutex> lk(server_mtx);
        if (!server_cv.wait_for(lk, std::chrono::seconds(5), []{ return server_is_listening; }))
            throw std::runtime_error("Server not listening after 5 seconds!");
        }
        //

        socket_ptr sConnection(::socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
        if (!sConnection)
            WinSockError("socket");

        SOCKADDR_IN addr = {};
        addr.sin_family = AF_INET;
        addr.sin_addr.s_addr = ::inet_addr("127.0.0.1");
        addr.sin_port = ::htons(server_port);

        if (::connect(sConnection, (SOCKADDR*)&addr, sizeof(addr)) == SOCKET_ERROR)
            WinSockError("connect");

        std::cout << "[Client] Connected!" << std::endl;

        try
        {
            std::string MOTD;
            while (true)
            {
                MOTD = recvStr(sConnection);
                std::cout << "[Client] MOTD: " << MOTD << std::endl;
            }
        }
        catch (const connection_closed &)
        {
        }
        catch (const winsock_error &e)
        {
           std::cerr << "[Client] Server error: " << e.errCode << " on WinSock function: " << e.funcName << std::endl;
        } 

        std::cout << "[Client] Disconnected!" << std::endl;
    }
    catch (const winsock_error &e)
    {
        std::cerr << "[Client] Error: " << e.errCode << " on WinSock function: " << e.funcName << std::endl;
    }
    catch (const std::exception &e)
    {
        std::cerr << "[Client] Unexpected Error! " << e.what() << std::endl;
    }

    std::cout << "[Client] Stopped!" << std::endl;
}

int main()
{
    WSADATA wsaData;

    int ret = ::WSAStartup(MAKEWORD(2, 1), &wsaData);
    if (ret != 0)
    {
        std::cerr << "WinSock Startup failed with error: " << ret << std::endl;
        return -1;
    }

    std::cout << "WinSock Startup successful" << std::endl;

    std::thread t[num_threads];

    t[0] = std::thread(server_thread);
    t[1] = std::thread(client_thread);

    for (int i = 0; i < num_threads; ++i) {
        t[i].join();
    }

    ::WSACleanup();
    return 0;
}