sccs
sccs

Reputation: 1163

When to use constructor and destructor, especially in containers and across threads

I have a multi-threaded program (a client-server one, but not necessarily relevant in this question) where multiple threads access global queues. There are two queues: msgs_inc and clients_msg which is of type queue<msgInfo>, where msgInfo is my class.

The first thread, receives a message from a client, does the following (relevant snippet):

msgInfo message_processed(message_fr_client); //line 1: takes in string 
msgs_inc.push(message_processed); //line 2

The second thread is supposed to retrieve from msgs_inc, process them, then push into clients_msg.

msgInfo msg_from_queue(msgs_inc); //line 3: retrieve from front of queue
msgs_inc.pop(); //line 4
clients_msg.push(msg_from_queue); //line 5

The third thread retrieves from clients_msg, after which it is no longer required.

msgInfo msg_from_queue(clients_msg); //line 6: retrieve from front of queue
clients_msg.pop(); //line 7

My question is:

  1. In line 3, is this constructor -detailed below- known as a copy constructor vs. a standard constructor?
  2. Am I wrong in 'instantiating' msgInfo twice i.e. once before pushing it in, and then again before retrieving it? Am I supposed to use a pointer or something else instead? It feels like it might inefficient, but I don't know another method.
  3. When I do apply a destructor? Do I only apply it after line 7, when I no longer require it, or do I need to apply a destructor again at line 4 since I've already created another instance of msgInfo with the information?

My apologies for this tediousness - I can't find information about this and can't piece a concrete conclusion together.


This is my class:

class msgInfo
{
public:
    msgInfo();
    msgInfo(std::string); //creating instance fr string rxed fr client
    msgInfo(std::map<int, std::map<int, std::queue<msgInfo>>>, int, int); //creating instance for string to be sent to client

    ~msgInfo();

private:
    int source_id;
    int dest_id;
    int priority;
    std::string payload;
    std::list<int> nodePath;
};

The constructor used in line 3 and line 6:

msgInfo::msgInfo(std::queue<msgInfo> outgoing_msg)
{
    source_id = outgoing_msg.front().source_id;
    dest_id = outgoing_msg.front().dest_id;
    priority = outgoing_msg.front().priority;
    payload = outgoing_msg.front().payload;
    nodePath = outgoing_msg.front().nodePath;
}

Upvotes: 0

Views: 189

Answers (1)

James Kanze
James Kanze

Reputation: 153977

I would not use the constructor from a queue. This is confusing. It's far better (and clearer) to write:

msgInfo msg_from_queue( msgs_inc.front() );

This invokes the copy constructor (since it makes a copy); in your case, the copy constructor provided by the compiler is sufficient.

As for instantiating msgInfo twice, you're actually instantiating it more than that, since it gets copied from your original instance into the queue, then from the queue to the second thread. There's nothing wrong with this. C++ is designed to privilege the use of value semantics, and doing otherwise requires special handling. If it later turns out that the copying is a performance bottleneck, you can fix it then. (Probably by making copy less expensive, rather than by moving to pointers. Get rid of that std::list, for example.)

If you do use pointers, you have to consider issues of ownership. You cannot just pass the address of a local variable. And having two or more threads access the same data is illegal if anyone is modifying the data.

As for your last question: you don't call (I presume that's what you mean by "apply") the destructor (at least when using value semantics). The compiler calls it automatically for local variables, and the underlying container of the queue calls it when the object is removed from the queue. For that matter, you don't call the constructor either: when you define an object, the compiler calls it for you, and when the object is copied into the queue, the queue performs the copy.

Finally, you've not shown any of your synchronization logic. I presume that this is simply because you're removed it to simplify the demonstration, and that you do know that you need to synchronize access as soon as there are multiple threads.

Upvotes: 1

Related Questions