Hey guys. I'm a little bit new to winsock programming in C++ and ran into an issue. The server works fine with one client up. And once I get another client up, the server can receive one or two messages from the first client before it ultimately is no longer receiving messages from that client, but is able to receive unlimited messages from the 2nd client. This holds true if I were to open a 3rd client, the serve is able to pick up a message or two from the 2nd client until it no longer receives messages from that client, while its able to receive unlimited messages from the 3rd client. I can't for the life of me figure out whats going on but its the closest I've ever gotten to a simple chat program with this code I whipped up. Bare in mind I eliminated the error handling to make it easier to understand for now until I get it working.

Server code:

#include <iostream>
#include <winsock2.h>
#include <ws2tcpip.h>#
include <string>
using namespace std;
#pragma comment (lib, "Ws2_32.lib")
#define IP_ADDRESS "192.168.56.1"
#define DEFAULT_PORT "3504"
#define DEFAULT_BUFLEN 2024
const char option_value = 1;
//Function Prototypes
DWORD WINAPI ProcessClient(LPVOID Parameter);
int main();
WSADATA wsaData;
struct addrinfo hints;
struct addrinfo *server = NULL;
SOCKET server_socket = INVALID_SOCKET;
SOCKET client_socket= INVALID_SOCKET;
int num_clients = 0;
string msg = "";
char tempmsg[DEFAULT_BUFLEN] = "";

DWORD WINAPI ProcessClient(LPVOID Parameter)
{	
    //Session	
    cout << "Client #" << num_clients << " Accepted" << endl;	
    num_clients++;	
    while (1)	
    {			
        memset(tempmsg, 0, DEFAULT_BUFLEN);			
        if (client_socket != 0)			
        {				
            int iResult = recv(client_socket, tempmsg, DEFAULT_BUFLEN, 0);
            if (iResult > 0)				
            {					
                if (strcmp("", tempmsg))
		    msg = tempmsg;					
                cout <<  msg.c_str() << endl << endl;
            }		
        }	
    }	
    return 0;
}

int main()
{	
    //Initialize Winsock	
    cout << "Intializing Winsock..." << endl;	
    WSAStartup(MAKEWORD(2, 2), &wsaData);	

    //Setup hints	
    ZeroMemory(&hints, sizeof(hints));	
    hints.ai_family = AF_INET;	
    hints.ai_socktype = SOCK_STREAM;	
    hints.ai_protocol = IPPROTO_TCP;	
    hints.ai_flags = AI_PASSIVE;	

    //Setup Server	
    cout << "Setting up server..." << endl;	
    getaddrinfo(static_cast<LPCTSTR>(IP_ADDRESS), DEFAULT_PORT, &hints, &server);
	
    //Create a listening socket for connecting to server	
    cout << "Creating server socket..." << endl;	
    server_socket = socket(server->ai_family, server->ai_socktype, server->ai_protocol);	

    //Setup socket options	
    setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, &option_value, sizeof(int)); //Make it possible to re-bind to a port that was used within the last 2 minutes	
    setsockopt(server_socket, IPPROTO_TCP, TCP_NODELAY, &option_value, sizeof(int));     //Used for interactive programs	

    //Assign an address to the server socket.	
    cout << "Binding socket..." << endl;	
    bind(server_socket, server->ai_addr, (int)server->ai_addrlen);	

    //Listen for incoming connections.	
    cout << "Listening..." << endl;	
    listen(server_socket, SOMAXCONN);	
    while (1)	
    {		
        while (client_socket = accept(server_socket, NULL, NULL))		
        {			
            DWORD threadID;			
            CreateThread(NULL, 0, ProcessClient, (LPVOID)client_socket, 0, &threadID);		    
        }	
    }	

    //Close listening socket	
    closesocket(server_socket);	

    //Close client socket	
    closesocket(client_socket);	

    //Clean up Winsock	
    WSACleanup();	
    cout << "Program has ended successfully" << endl;	
    system("pause");	
    return 0;
}

The client code isnt necessary, as the problem lies in here.

In attempt two to try and get it working, I tried making the client_socket an array. Although it sort of works, it works in a very bad way. For example, if I have client 1 and client 2 log on afterwards. The server can receive a message from client 1 but cant from client two until it has received a message from client 1, and vice versa. As though it gets stuck in recv() in the middle of a for loop in the thread. I have no idea on how to fix any of this and any help will be appreciated. Thanks in advance.

http://www.gamedev.net/blog/1983/entry-2260509-heres-some-code-it-doesnt-work-whats-wrong/

In addition to the blog article which Glass_Knife linked to (which is a good article, and you should definitely read and follow), the first thing I noticed when looking at your code is that you are doing multithreaded programming using global variables.

This is very very bad. It is likely at least part of your networking problem, and even if it somehow isn't, it will become the source of huge numbers of other (hard to diagnose) problems if you extend this sort of code to do anything else.

You might want to take a step back and read up a bit more about multithreaded programming, data isolation, thread synchronization, and thread-safe data structures before going much further. Alternately, you should avoid using things like CreateThread() entirely and just stick with single-threaded programming (using select(), etc) for the time being (it may seem more cumbersome initially, but it will save you a ton of grief in the long run if you're not already familiar with how to work with threads safely. It also often makes debugging a lot easier when things don't work right)..

Well im not a beginner programmer. I actually been programming since i was 10. Im 33 now and do it as a hobby and in school for it. Im just new to network programming. I figured the code displayed was so small, and it is, that i could put the whole thing.

CreateThread(NULL, 0, ProcessClient, (LPVOID)client_socket, 0, &threadID);

int iResult = recv(client_socket, tempmsg, DEFAULT_BUFLEN, 0);

This.

You are accepting new sockets client_socket but the next new socket overwrites client_scoekt.

This would not be a problem if you actually used the LPVOID parameter you passed to ProcessClient() as the socket instead of using the global client_socket.

So basically use a local variable inside of ProcessClient and assign the parameter to it.

Like

DWORD WINAPI ProcessClient(LPVOID Parameter)
{

SOCKET local_socket = *( (SOCKET *)&Parameter ); //someone will criticize my weird casting LOL
//Session
cout << "Client #" << num_clients << " Accepted" << endl;
num_clients++;
while (1)
{
memset(tempmsg, 0, DEFAULT_BUFLEN);
if (local_socket != 0)
{
int iResult = recv(local_socket, tempmsg, DEFAULT_BUFLEN, 0);
if (iResult > 0)
{

//OH, please be sure to use the SAFE string functions!!!!!! strncmp()
if (strcmp("", tempmsg))
msg = tempmsg;
cout << msg.c_str() << endl << endl;
}
}
}
return 0;
}

Interesting. Ill take a whack at it when i get home. Also i may have found a proper way to handle an array of client sockets from the gamedev article:

http://www.gamedev.net/page/resources/_/technical/multiplayer-and-network-programming/winsock2-for-games-r1059

I also wouldnt mind seeing examples of other methods, such as select(), asyncronous sockets, etc., because finding a simple example on the web is tough. The code is either too complex, unreadable, or broken up into so many separate files and classes that its hard to see how it works. I understand networking is a beast, but it doesnt need to be impossible to learn.

SOCKET local_socket = *( (SOCKET *)&Parameter ); //someone will criticize my weird casting LOL

Try this:

SOCKET local_socket = *(reinterpret_cast<SOCKET *>(&Parameter));

:P

Holy cow it works!!! Thanks a bunch!

I also added a structure in my server so I can add a unique id for each client so I know which client window is talking. This is the simplest chat program I've ever seen and created for that matter. Not to mention the code can easily be copy and pasted without worrying about adding libraries. Heres the new code I have for both the server and client:

Server:

#include <iostream>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <string>
using namespace std;

#pragma comment (lib, "Ws2_32.lib")

#define IP_ADDRESS "192.168.56.1"
#define DEFAULT_PORT "3504"
#define DEFAULT_BUFLEN 2024

struct client_type
{
    int id = 0;
    SOCKET client_socket = INVALID_SOCKET;
};

const char option_value = 1;
const int maxClients = 5;

//Function Prototypes
DWORD WINAPI ProcessClient(LPVOID Parameter);
int main();

WSADATA wsaData;

struct addrinfo hints;
struct addrinfo *server = NULL;

SOCKET server_socket = INVALID_SOCKET;
client_type client[maxClients];

int num_clients = 0;

string msg = "";
char tempmsg[DEFAULT_BUFLEN] = "";

DWORD WINAPI ProcessClient(LPVOID Parameter)
{
    client_type local_socket;
    local_socket = *(static_cast<client_type *>(Parameter));

    //Session
    if (num_clients < maxClients)
    {
        cout << "Client #" << local_socket.id << " Accepted" << endl;
        num_clients++;
    
        while (1)
        {
                memset(tempmsg, 0, DEFAULT_BUFLEN);

                if (local_socket.client_socket != 0)
                {
                    int iResult = recv(local_socket.client_socket, tempmsg, DEFAULT_BUFLEN, 0);

                    if (iResult > 0)
                    {
                        if (strcmp("", tempmsg))
                            msg = tempmsg;

                        cout << "Client #" << local_socket.id << ": " << msg.c_str() << endl;
                    }
                    else
                    {
                        cout << "Client #" << local_socket.id << " Disconnected" << endl;
                        closesocket(local_socket.client_socket);
                        num_clients--;
                        if (num_clients <= 0) num_clients = 0;
                        break;
                    }
            }
        }
    }
    else
    {
        cout << "The server has reached the maximum number of clients." << endl;
    }

    return 0;
}

int main()
{
    //Initialize Winsock
    cout << "Intializing Winsock..." << endl;
    WSAStartup(MAKEWORD(2, 2), &wsaData);

    //Setup hints
    ZeroMemory(&hints, sizeof(hints));
    hints.ai_family = AF_INET;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;
    hints.ai_flags = AI_PASSIVE;

    //Setup Server
    cout << "Setting up server..." << endl;
    getaddrinfo(static_cast<LPCTSTR>(IP_ADDRESS), DEFAULT_PORT, &hints, &server);

    //Create a listening socket for connecting to server
    cout << "Creating server socket..." << endl;
    server_socket = socket(server->ai_family, server->ai_socktype, server->ai_protocol);

    //Setup socket options
    setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, &option_value, sizeof(int)); //Make it possible to re-bind to a port that was used within the last 2 minutes
    setsockopt(server_socket, IPPROTO_TCP, TCP_NODELAY, &option_value, sizeof(int)); //Used for interactive programs

    //Assign an address to the server socket.
    cout << "Binding socket..." << endl;
    bind(server_socket, server->ai_addr, (int)server->ai_addrlen);

    //Listen for incoming connections.
    cout << "Listening..." << endl;
    listen(server_socket, SOMAXCONN);

    while (1)
    {
        client[num_clients].client_socket = accept(server_socket, NULL, NULL);
        client[num_clients].id = num_clients;
        DWORD threadID;
        CreateThread(NULL, 0, ProcessClient, (LPVOID)&client[num_clients], 0, &threadID);
    }

    //Close listening socket
    closesocket(server_socket);

    //Close client socket
    for (int i = 0; i < maxClients; i++)
    {
        closesocket(client[i].client_socket);
    }

    //Clean up Winsock
    WSACleanup();
    cout << "Program has ended successfully" << endl;
    system("pause");
    return 0;
}

Client:

#include <winsock2.h>
#include <ws2tcpip.h>
#include <iostream>
#include <string>
using namespace std;

#pragma comment (lib, "Ws2_32.lib")

#define DEFAULT_BUFLEN 512            
#define IP_ADDRESS "192.168.56.1"
#define DEFAULT_PORT "3504"

WSADATA wsaData;
SOCKET ConnectSocket = INVALID_SOCKET;
struct addrinfo *result = NULL,
                *ptr = NULL,
                hints;
char recvbuf[DEFAULT_BUFLEN];
int iResult = 0;
int recvbuflen = DEFAULT_BUFLEN;
string msg = "";

int main()
{

    cout << "Starting Client...\n";

    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0) {
        cout << "WSAStartup failed with error: " << iResult << endl;
        return 1;
    }

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

    cout << "Connecting...\n";

    // Resolve the server address and port
    iResult = getaddrinfo(static_cast<LPCTSTR>(IP_ADDRESS), DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 ) {
        cout << "getaddrinfo failed with error: " << iResult << endl;
        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) {
            cout << "socket failed with error: " << WSAGetLastError() << endl;
            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) {
        cout << "Unable to connect to server!\n";
        WSACleanup();
        return 1;
    }

    cout << "Successfully Connected\n\n";

    // Receive until the peer closes the connection
    do {
        cout << "Type in something:\n";
        msg = "";
        getline(cin,msg);
        //iResult = recv(ConnectSocket, recvbuf, recvbuflen, 0);
        iResult = send(ConnectSocket, msg.c_str(), strlen(msg.c_str()), 0);
        if ( iResult > 0 )
            cout << endl;
        else if ( iResult == 0 )
            cout << "Connection closed\n\n";
        else
            cout << "recv failed with error: " << WSAGetLastError() << endl;

    } while (iResult > 0);

    // shutdown the connection since no more data will be sent
    iResult = shutdown(ConnectSocket, SD_SEND);
    if (iResult == SOCKET_ERROR) {
        cout << "shutdown failed with error: " << WSAGetLastError() << endl;
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }
    // cleanup
    closesocket(ConnectSocket);
    WSACleanup();

    return 0;
}

SOCKET local_socket = *( (SOCKET *)&Parameter );

Try this:

 
SOCKET local_socket = *(reinterpret_cast<SOCKET *>(&Parameter));

That's no better than the previous code (which also would work, just like this.) What you really want is this:

SOCKET local_socket = (SOCKET)Parameter;

There is no reason to take the address of the value just to derefenrece it again.
In fact, if sizeof(Parameter) > sizeof(SOCKET), the initial address-based casts may fail, whereas the plain value cast will succeed.

I just realized that i didnt broadcast the clients' messages to the other clients lol. /Facepalm.

So from what i know, you cant broadcast using tcp. Only with udp. Does that mean the server should use a udp socket for receiving and broadcasting to the other clients, while the clients use tcp to send directly to the server?

UDP broadcast will not work across the internet.
The way to "broadcast" a message is to enqueue a copy of the message to each of the connected clients.