Thread contemporary access to variables and synchronization mechanisms

I want to understand if the foll开发者_如何学Cowing code is correct

#include <iostream>
#include <boost/thread/thread.hpp>
#include <vector>

using namespace std;
using namespace boost;

class background_task
{
private:
    std::vector<int> numbers;

public:
    background_task()
    {
        i=0;
        numbers.assign(6000,0);
    }

    void someLongComputation()
    {
        while (++i<200)
        {
            //boost::mutex::scoped_lock(formutex);
            // cout << "Thread inside: i= " << this->i << endl;
            numbers.at(0)=i;
            cout << "Thread inside numbers= " << numbers.at(0) << endl; 
            boost::this_thread::sleep(boost::posix_time::milliseconds(100));
        }
    }

    std::vector<int>& getNumbers()
    {
        return numbers;
    }

    int i;
};

background_task f;

void valuePicker()
{
    int j=0;
    while ( (j++) < 20 )
    {
        boost::this_thread::sleep(boost::posix_time::milliseconds(1000));
        cerr << "First number= " << f.getNumbers().at(0) << endl;
    }
}

int main(void)
{
    boost::thread comp(boost::bind(&background_task::someLongComputation, &f));
    boost::thread value(valuePicker);

    comp.join();
    return 0;
}

This code should start two thread: one that makes someLongComputation (I added a timer sleeper to simulate the loooong computations which in fact are very short) and another thread which accesses at a different frequency to data contained in the computation thread.

I'm wondering now if this approach is correct, because it seems to me no mutexes are required, I would ask you how to make this code more thread-safe and correct, because I suppose I'm missing something...

Is it correct to pass a non-const reference or should be more safe to make it const? Data created should never be written, only read...

Thanks! I hope with this thread to finally fix my beginners doubts on multithreading things...

Reading and writing to the shared array numbers is not thread safe. The artificial sleep that you have inserted are likely masking the issue if this code is actually running successfully.

You need to have proper protection around the numbers vector, which is very difficult given the exact example because you expose this vector to clients through the public function getNumbers(). Doing so means you cannot necessarily control how clients of the background_task actually interact with the data.

I would add a boost::mutex (as you have already commented out in the long computation function), and lock it in the long computation function (only when accessing the array to keep the scope of the lock small). I would also remove the getNumbers() function and replace it with a function to get a value from the array at a specific index, such as int getNumberAt(size_t idx); and lock the boost::mutex there as well.