1

I have an environment modelled by lines and points packed in two std::vector. I want to calculate a field generated by this environnement. I multithreaded the process. As the environment is totally defined at the begining, threads should only read on it so I don't use any syncrhonisation as discribed here and there.

The problem comes now : when I iterate through the lines in the environement, I have two different behaviours depending if I use the c++11 range-based for statment or if I use a more common for statement with iterators. It seems that the range-based for isn't thread-safe and I'm wondering why?

If I'm not right assuming that, it might significate I have a more deep problem that may reappear latter.

Here is a piece of code, the first worker seems to work, the second provoke a segfault.

Worker::worker(Environement const* e, int id):threadId(id),env(e)
{
}

// worker that seems to do his job.
void Worker::run() const
{
    cout<<"in thread n "<<_threadId<<endl;
    vector<Line> const* lines = &env->_lines;

    for(std::vector<Line>::const_iterator it = lines->begin() ;it != lines->end(); ++it ){
        it->hello();
    }
}

// create a segfault
void Worker::run2() const
{
    cout<<"in thread n "<<_threadId<<endl;
    vector<Line> const& lines = env->_lines;

    for(auto it : lines){
        it.hello();
    }
}

The simplified structure of data if needed:

struct Line
{
    void hello() const {std::cout<<"hello"<<std::endl;}
}

struct Environment
{
    std::vector<Line> _lines;
    std::vector<Point> _points;
}
Community
  • 1
  • 1
Simon
  • 11
  • 4
  • 3
    `auto it : lines` You declare a variable to store the current *element* in a range-based for loop, not a variable to store the current iterator. Hence, this declaration *copies* elements from the vector. Typically, you can use `auto const& line : lines` (à la "for each line in lines") if your loop does not modify the elements. – dyp Feb 21 '15 at 22:30
  • There is also a wrinkle that std::cout is a global resource, so writing to it from multiple threads requires access to it to be synchronised. That is true whether or not the containing loop construct is appropriate. – Rob Feb 21 '15 at 22:42
  • As an aside, your `Line` copy ctor shoukd be rewritten to be thread safe. `Line(Line const&)` – Yakk - Adam Nevraumont Feb 21 '15 at 22:54
  • 1
    @Rob As long as it's synchronized with stdio, there's no data race, though you can get interleaving characters. – T.C. Feb 21 '15 at 23:09
  • [Here's an MCVE that includes all the code you posted in your question, along with my guess at the machinery needed to make a complete compilable program](http://coliru.stacked-crooked.com/a/5662a35a16e8e289). Notice that it doesn't demonstrate the issue you describe in your question. You got lucky this time, but someone probably won't always be able to guess what the problem is. In the future, you should include a [proper MCVE](http://stackoverflow.com/help/mcve) in your questions. (80% of the time reducing your program to a MCVE will show you what the problem is and save you/us the trouble). – Casey Feb 22 '15 at 04:33

0 Answers0