Does this model need a mutex?

I'm designing a program with a server that let's two clients communicate. There's one client that executes commands and another that makes the other client execute them.

To make this work, I have two threads: one thread for the controlled-client, another for the controller-client.

The controlled-client always stays connected, has a vector with tasks and executes these tasks if a task is added. It has an infinite while loop which is stopped when the connection is closed.

The controller-client adds tasks to the vector.

Now since there is an vector shared between two threads a race condition may occur, but since one thread only adds objects and the other only pops objects, is that necessary? I tried to make a flowchart with this problem, but maybe it isn't clear. I don't really know how to create a flowchart:

I'm using std::vector <Cu开发者_如何转开发stomClass> from C++ to realize this.

Thanks in advance,

ief2

EDIT: Additional Question: Does the vector.size() call need a mutex?

Yes, this needs a mutex.

vector.pop_front() and vector.push_back() executing at the same time would create all sorts of mess - assuming the nonstandard vector.pop_front() does what the name implies.

Side note: use a queue or a list rather than a vector

You should control access to the vector. You don't want to be trying to push and pop simultaneously.

Yes you need a mutex. On a multiple processor machine both threads could be trying to push and pop at the same time. A condition variable to indicate when there is something to process might be a good idea too.

I'm wondering how the controlled-client is implemented. Is it a loop that keeps checking the task queue? If it is the case and you believe that efficiency is more important, you can implement the task queue as a singly-linked list and still have a thread-safe and lock-free implementation. All you need is a dumb node in the list and the head is pointed to this dumb node. The empty list is represented as the dumb node's next field is null. When you pop, you just move the head from the current dumb node to the next node which becomes the new dumb node. When you push, you just append the task, since the list is not empty(contains at least a dumb node). This only works for two-thread scenario.

STL containers are not thred-safe. So yes you need a locking mechanism (i.e a mutex) to make the container thread-safe.

You may read this SO question to see an example for wrapping an STL queue for thread-safety.

Flow charts usually aren't a good idea.

Synchronization is required. Inserting or erasing from an std::vector in multiple threads is not safe.

What happens if you one thread pushes and the other thread pops at the same time, but the push causes the vector's internal array to be reallocated? The popping thread is accessing some memory that's possibly no longer in use.

A similar situation can occur if one thread is reading and the other thread is pushing. If the push causes the vector to reallocate, the read is now accessing memory that's most likely no longer valid.

No you don't need a mutex, but some kind of synchronisation is necessary. I would recommend not using infinite timed loops, and would use events instead. Have the producer signal if it has finished writing to the shared collection, and have the consumer signal when it is finished reading the commands (don't start executing the commands while preventing the producer from writing).

This stop useless CPU cycles of checking for commands in an empty vector.