Passing data to another thread in a C++ winsock app

Question

So I have this winsock application (a server, able to accept multiple clients), where in the main thread I setup the socket and create another thread where I listen for clients (listen_for_clients function).

I also constantly receive data from a device in the main thread, which I afterwards concatenate to char arrays (buffers) of Client objects (BroadcastSample function). Currently I create a thread for each connected client (ProcessClient function), where I initialize a Client object and push it to a global vector of clients after which I send data to this client through the socket whenever the buffer in the corresponding Client object exceeds 4000 characters.

Is there a way I can send data from the main thread to the separate client threads so I don't have to use structs/classes (also to send a green light if I want to send the already accumulated data) and also if I'm going to keep a global container of objects, what is a good way to remove a disconnected client object from it without crashing the program because another thread is using the same container?

struct Client{
    int buffer_len;
    char current_buffer[5000];
    SOCKET s;
};

std::vector<Client*> clientBuffers;

DWORD WINAPI listen_for_clients(LPVOID Param) 
{
    SOCKET client;
    sockaddr_in from;
    int fromlen = sizeof(from); 
    char buf[100];
    while(true)
    { 
        client = accept(ListenSocket,(struct sockaddr*)&from,&fromlen);
        if(client != INVALID_SOCKET)
        {
            printf("Client connected\n");
            unsigned dwThreadId;
            HANDLE hThread = (HANDLE)_beginthreadex(NULL, 0, &ProcessClient, (void*)client, 0, &dwThreadId);
        }
    }

    closesocket(ListenSocket); 
    WSACleanup(); 
    ExitThread(0);
}

unsigned __stdcall ProcessClient(void *data)
{
    SOCKET ClientSocket = (SOCKET)data;
    Client * a = new Client();
    a->current_buffer[0] = '\0';
    a->buffer_len = 0;
    a->s = ClientSocket;
    clientBuffers.push_back(a);

    char szBuffer[255];

    while(true)
    {   
       if(a->buffer_len > 4000)
       {
          send(ClientSocket,a->current_buffer,sizeof(a->current_buffer),0);
          memset(a->current_buffer,0,5000);
          a->buffer_len = 0;
          a->current_buffer[0] = '\0';
       }
    }
    exit(1);
}

//function below is called only in main thread, about every 100ms
void BroadcastSample(Sample s)
{
    for(std::vector<Client*>::iterator it = clientBuffers.begin(); it !=  clientBuffers.end(); it++)
    {
        strcat((*it)->current_buffer,s.to_string);
        (*it)->buffer_len += strlen(s.to_string);
    }
}

What are the per-socked threads doing for you? does the `send(socked,buffer,size,0)` call take a long time? I don't think you're gaining anything by having those threads. — Jfevold, May 03 '16 at 01:45
@Jfevold: as a general rule, if you have multiple clients all I/O should either be done in separate threads or asynchronously. Otherwise a single misbehaving client could potentially bring everything to a screaming halt. — Harry Johnston, May 03 '16 at 01:46
If you're using a recent enough version of C++ you should probably use its threading support, which no doubt includes thread-safe queues and the like. If you want to use the Windows API, [interlocked singly linked lists](https://msdn.microsoft.com/en-us/library/windows/desktop/ms684121(v=vs.85).aspx) might be a suitable option. — Harry Johnston, May 03 '16 at 01:49
Your are rapidly reaching the point where you have too many complex and asynchronous events occurrring to keep sane track of in multiple threads. Either the logic will beak you, or the deadlocks will. With your requirements, I would be heading for a message-passing design with just one thread dedicated to running a state-machine and maintaining the list/s of clients/buffer/whatever. All other threads would queue event object instances to it. Such a design may well introduce more overhead etc, but has the advantage of being likely to be actually debuggable and race/deadlock-free. — Martin James, May 03 '16 at 09:58
You can, of course, use lots of locks and have threads performing operations in an asynchronous fashion on multiple data items. With care and luck, that may work-ish, but it's like the weeping angels: one blink and you will get sucked down into a nightmarish hell of unpredictable, morphing failures. — Martin James, May 03 '16 at 10:04

score 1 · Accepted Answer · edited May 23 '17 at 11:52

This link has some Microsoft documentation on MS-style mutexes (muticies?).

This other link has some general info on mutexes.

Mutexes are the general mechanism for protecting data which is accessed by multiple threads. There are data structures with built-in thread safety, but in my experience, they usually have caveats that you'll eventually miss. That's just my two cents.

Also, for the record, you shouldn't use strcat, but rather strncat. Also, if one of your client servicing threads accesses one of those buffers after strncat overwrites the old '\0' but before it appends the new one, you'll have a buffer overread (read past end of allocated buffer).

Mutexes will also solve your current busy-waiting problem. I'm not currently near a windows compiler, or I'd try to help more.

Passing data to another thread in a C++ winsock app

1 Answers1