Two threads are each getting their own copy of object stored in map

Question

Title sort of says it all, I thought storing an object in an object container would allow for easy cross-threaded class-member access, because it essentially stores the object in memory space that is managed by the object container, in this case a map. Is this incorrect? Because the following is happening;

Client class:

class Client
{
public:
    Client(std::string clientID,SOCKET sock,bool quit);
    boost::thread_group *group;
    /*CUT*/
    std::string clientID;
    std::deque<std::string> snapShotsQueue;
    SOCKET sock;
    bool quit;
    void sendMessage(std::string);
    void threadSend(Client *client);
    void checksnapshots();
    /*CUT*/
};

The map

typedef std::map<std::string, Client> clientMap;
clientMap clientmap;

1. Server starts
2. Boost thread is started(1) that is constantly checking values. The idea is that if certain things happen on the server, all qualifying clients get notified. To make this happen, the message is added to the client class's deque.
3. Server is constantly accepting new client connections, which each get their own thread (ClientThread).
4. Client object is created inside that thread(2)
5. Client class' construct starts yet another thread that is constantly sending messages stored in the client-class' object's deque (3).

(1) The boost thread that is created in main()

void alwaysWatching()
{
    while(1)
    {
        /*CUT*/
        /* When something that needs to be communcated happens, a message will be formed and stored in the string "thaString" and sent to, in this case, all clients*/  
        for (clientMap::iterator it2 = clientmap.begin(); it2 != clientmap.end(); ++it2)
        {
                it2->second.snapShotsQueue.push_back(thaString); //Add to client's deque
                //Check how many items are in deque
                std::cout << "There are now ";
                it2->second.checksnapshots();                           
                std::cout << "Snapshots waiting according to outside watcher" << std::endl;
        }
        /*CUT*/
    }
}

(2) Creating and adding the client object to the map

DWORD WINAPI ClientThread(LPVOID lpParam)
{
    SOCKET        sock=(SOCKET)lpParam;

    /*CUT*/

    std::string clientID = "";
    std::stringstream ss;
    ss << lpParam; //Socket = clientID
    clientID = ss.str();

    Client client(clientID,sock,false); //Create object for this client

    while(1) //This thread is constantly waiting for messages sent by the client
    {
        /*CUT*/
        //Add clientID to map of clients
        if(clientAdded == false)
        {
            /*CUT*/
            clientmap.insert(std::pair<std::string,Client>(clientID,client));
            clientAdded = true;
            /*CUT*/
        }
        /*CUT*/
    return 0;
}

(3) The thread that sends all messages in the deque to the client

//Struct used to create the thread that will keep on sending messages in deque to the client
struct messageSender
{
    messageSender(Client *client) : client(client) { }

    void operator()()
    {
        client->threadSend(client);
    }
    Client *client;
};

//Client constructor
Client::Client(std::string clientIDs,SOCKET socks,bool quits)
{
    /*CUT*/
    this->group = new boost::thread_group; //Create boost thread group (for later, possibly)
    messageSender startit(this); //Prep new thread for sending snapshot updates
    group->create_thread(startit); //Start new thread for snapshot updates
    /*CUT*/
}

//The actual function that constantly loops through the deque
void Client::threadSend(Client *client)
{
    /*CUT*/
    while(1)
    {
        /*CUT*/
            std::cout << "There are now ";
            client->checksnapshots();
            std::cout << "Snapshots waiting according to class thread queue processor" << std::endl;
        /*CUT*/

        unsigned int i;
        for(i=0; i < client->snapShotsQueue.size(); i++)
        {
            std::string theString;
            theString = client->snapShotsQueue.front();  // this gets the front of the deque
            client->snapShotsQueue.pop_front();             // this removes the front of the deque

            std::cout << "sending: " << theString << std::endl;
            client->sendMessage(theString);
        }
    }
}

As you can see, I added a piece of code that counts the deque in both the thread outside of the class, as well as inside of the class. They both report different counters and the messages from the thread outside of the class aren't being sent.

So it seems the watcher thread (1) has its own instance of the Client object even though it's stored inside of the map. Or something in that direction.

I'm probably doing something wrong pointer-wise. Any ideas?

Answer 1

Depending on your problem you may really want to store pointers instead of objects in your map as ruakh proposes - just be careful there or maybe use shared_ptrs.

Or if you're fine with storing the object in the map itself, you can just access it by reference - in that case no copy is created but you don't have to deal with memory allocation:

Simple example:

std::map<std::string, int> m;
m["Test"] = 5;
int& val = m["Test"];
int& val2 = m["Test"];
val = 10;
printf("%d\n", val2); // prints 10, not 5.

Answer 2

You're copying Clients into the map, yes, but then whenever you read them out, you're implicitly creating new Clients by copying the ones in the map. The new copies will have separate queues of snapshots.

You probably want to be using either std::map<std::string, Client *> or std::map<std::string *, Client *>, and allocating all of your Clients using new Client(...) (with corresponding deletes). Then, for each client you put in the map, there can be just a single Client instance with multiple copies of pointers to it.