Data Race problem

Discussion in 'C++' started by Saeed Amrollahi, Dec 12, 2012.

  1. Hi

    I have the following very simple multi-thread program:
    #include <thread>
    #include <mutex>

    int var = 0;
    void incr()
    {
    std::mutex m;
    m.lock();
    var++;
    m.unlock();
    }

    int main()
    {
    std::thread t{incr};
    std::mutex m;
    m.lock();
    ++var;
    m.unlock();
    t.join();

    return 0;
    }
    This is really simple program, and I think,
    I made a mutual exclusion, when two threads
    are writing shared variable (var).
    But, when I run valgrind cmd , it turns out there is
    data race. I used the following commands under Linux (GCC 4.7.0):
    $ g++ -std=c++11 -pthread -pedantic -Wall simple_data_race.c++
    $ valgrind -q --tool=helgrind ./a.out
    Here it is the <abbreviated> valgrind messages:
    <Message>
    ....
    Possible data race during write of size 8 at <address> by thread #1
    ....
    This conflicts with a previous write of size 8 by thread #2
    ....
    </Message>
    I struggled a few hours with such a simple code.
    Please shed some light.

    TIA,
    -- Saeed Amrollahi Boyouki
     
    Saeed Amrollahi, Dec 12, 2012
    #1
    1. Advertising

  2. On 12/12/2012 8:16 AM, Saeed Amrollahi wrote:
    > I have the following very simple multi-thread program:
    > #include <thread>
    > #include <mutex>
    >
    > int var = 0;
    > void incr()
    > {
    > std::mutex m;
    > m.lock();
    > var++;
    > m.unlock();
    > }
    >
    > int main()
    > {
    > std::thread t{incr};
    > std::mutex m;
    > m.lock();
    > ++var;
    > m.unlock();
    > t.join();
    >
    > return 0;
    > }
    > This is really simple program, and I think,
    > I made a mutual exclusion, when two threads
    > are writing shared variable (var).
    > But, when I run valgrind cmd , it turns out there is
    > data race. I used the following commands under Linux (GCC 4.7.0):
    > $ g++ -std=c++11 -pthread -pedantic -Wall simple_data_race.c++
    > $ valgrind -q --tool=helgrind ./a.out
    > Here it is the <abbreviated> valgrind messages:
    > <Message>
    > ...
    > Possible data race during write of size 8 at <address> by thread #1
    > ...
    > This conflicts with a previous write of size 8 by thread #2
    > ...
    > </Message>
    > I struggled a few hours with such a simple code.
    > Please shed some light.


    I thought that to prevent access to the same variable you're supposed to
    lock *the same* mutex in both threads, not two *different* ones. Pull
    the declaration/definition of 'm' out of 'incr' and 'main' into the
    global scope and try again.

    V
    --
    I do not respond to top-posted replies, please don't ask
     
    Victor Bazarov, Dec 12, 2012
    #2
    1. Advertising

  3. On Wednesday, December 12, 2012 5:02:19 PM UTC+3:30, Victor Bazarov wrote:
    > On 12/12/2012 8:16 AM, Saeed Amrollahi wrote:
    >
    > > I have the following very simple multi-thread program:

    >
    > > #include <thread>

    >
    > > #include <mutex>

    >
    > >

    >
    > > int var = 0;

    >
    > > void incr()

    >
    > > {

    >
    > > std::mutex m;

    >
    > > m.lock();

    >
    > > var++;

    >
    > > m.unlock();

    >
    > > }

    >
    > >

    >
    > > int main()

    >
    > > {

    >
    > > std::thread t{incr};

    >
    > > std::mutex m;

    >
    > > m.lock();

    >
    > > ++var;

    >
    > > m.unlock();

    >
    > > t.join();

    >
    > >

    >
    > > return 0;

    >
    > > }

    >
    > > This is really simple program, and I think,

    >
    > > I made a mutual exclusion, when two threads

    >
    > > are writing shared variable (var).

    >
    > > But, when I run valgrind cmd , it turns out there is

    >
    > > data race. I used the following commands under Linux (GCC 4.7.0):

    >
    > > $ g++ -std=c++11 -pthread -pedantic -Wall simple_data_race.c++

    >
    > > $ valgrind -q --tool=helgrind ./a.out

    >
    > > Here it is the <abbreviated> valgrind messages:

    >
    > > <Message>

    >
    > > ...

    >
    > > Possible data race during write of size 8 at <address> by thread #1

    >
    > > ...

    >
    > > This conflicts with a previous write of size 8 by thread #2

    >
    > > ...

    >
    > > </Message>

    >
    > > I struggled a few hours with such a simple code.

    >
    > > Please shed some light.

    >
    >
    >
    > I thought that to prevent access to the same variable you're supposed to
    >
    > lock *the same* mutex in both threads, not two *different* ones. Pull
    >
    > the declaration/definition of 'm' out of 'incr' and 'main' into the
    >
    > global scope and try again.
    >
    >
    >
    > V
    >
    > --
    >
    > I do not respond to top-posted replies, please don't ask


    very good point. I pulled out the mutex out of local scope and
    declared/defined after var. but the problem remains and I got
    similar messages from Valgrind.

    Thanks,
    -- Saeed
     
    Saeed Amrollahi, Dec 12, 2012
    #3
  4. On Wednesday, December 12, 2012 8:58:04 PM UTC+3:30, Paavo Helde wrote:
    > Saeed Amrollahi <> wrote in news:709c8109-d3f3-
    >
    > :
    >
    >
    >
    > > Hi

    >
    > >

    >
    > > I have the following very simple multi-thread program:

    >
    > > #include <thread>

    >
    > > #include <mutex>

    >
    > >

    >
    > > int var = 0;

    >
    > > void incr()

    >
    > > {

    >
    > > std::mutex m;

    >
    > > m.lock();

    >
    > > var++;

    >
    > > m.unlock();

    >
    > > }

    >
    >
    >
    > As Victor already pointed out, an automatic mutex variable makes no sense
    >
    > here as there is no way it can be seen from other threads and so it
    >
    > cannot protect anything.
    >
    >

    Right. I did what Victor wrote.
    >
    > Actually I had another question: why do you call lock() and unlock
    >
    > directly? This is not exception-safe and a sure recipe for deadlocks in
    >
    > long term. Instead you should always use a RAII lock object (in contrast
    >
    > to mutex, a lock object is indeed an automatic variable most of the
    >
    > time).
    >
    >
    >
    > It appears in C++11 the RAII lock object is called lock_guard:
    >
    >
    >
    > std::lock_guard<std::mutex> lock(m);
    >
    >

    Thanks. I know about unique_lock, lock_guard, RAII, condition variables.
    Actually, at first try, I wrote the program using unique_lock. but after
    I encountered the problem, I made the actual problem simpler little by little,
    to find the problem. BTW, it's not the main point. the problem remained.
    >
    > hth
    >
    > Paavo


    Regards,
    -- Saeed
     
    Saeed Amrollahi, Dec 12, 2012
    #4
  5. On 12/12/2012 1:01 PM, Saeed Amrollahi wrote:
    > [..] I pulled out the mutex out of local scope and
    > declared/defined after var. but the problem remains and I got
    > similar messages from Valgrind.


    It's possible that Valgrind has no way of knowing/checking that your
    increment is protected by a lock. Have you tried reading their manual
    or checking for a solution on the web?

    V
    --
    I do not respond to top-posted replies, please don't ask
     
    Victor Bazarov, Dec 12, 2012
    #5
  6. On Wednesday, December 12, 2012 9:51:28 PM UTC+3:30, Victor Bazarov wrote:
    > On 12/12/2012 1:01 PM, Saeed Amrollahi wrote:
    >
    > > [..] I pulled out the mutex out of local scope and

    >
    > > declared/defined after var. but the problem remains and I got

    >
    > > similar messages from Valgrind.

    >
    >
    >
    > It's possible that Valgrind has no way of knowing/checking that your
    >
    > increment is protected by a lock. Have you tried reading their manual
    >
    > or checking for a solution on the web?
    >
    >
    >
    > V
    >
    > --
    >
    > I do not respond to top-posted replies, please don't ask


    Sorry. I haven't read the manual yet. But my assumption simply
    is: if there are no Valgrind warning messages, there is no
    race condition problem!
    -- Saeed
     
    Saeed Amrollahi, Dec 12, 2012
    #6
  7. On Thursday, December 13, 2012 12:31:05 AM UTC+3:30, Luca Risolia wrote:
    > On 12/12/2012 14:16, Saeed Amrollahi wrote:
    >
    > > Hi

    >
    > >

    >
    > > I have the following very simple multi-thread program:

    >
    > > #include <thread>

    >
    > > #include <mutex>

    >
    > >

    >
    > > int var = 0;

    >
    > > void incr()

    >
    > > {

    >
    > > std::mutex m;

    >
    > > m.lock();

    >
    > > var++;

    >
    > > m.unlock();

    >
    > > }

    >
    > >

    >
    > > int main()

    >
    > > {

    >
    > > std::thread t{incr};

    >
    > > std::mutex m;

    >
    > > m.lock();

    >
    > > ++var;

    >
    > > m.unlock();

    >
    > > t.join();

    >
    > >

    >
    > > return 0;

    >
    > > }

    >
    > > This is really simple program, and I think,

    >
    > > I made a mutual exclusion, when two threads

    >
    > > are writing shared variable (var).

    >
    >
    >
    > In your simple case you can get rid of the mutex by declaring the shared
    >
    > variable as std::atomic_int.
    >
    >
    >
    > Anyway, if really want to protect shared data with *one* mutex, wrap it
    >
    > with a std::lock_guard or a std::unique_lock instead of using the
    >
    > mutex directly, as others have said. A std::lock_guard is faster and can
    >
    > be used in a local {} block in your main() as well:
    >
    >
    >
    > int main() {
    >
    > //...
    >
    > {
    >
    > std::lock_guard<std::mutex> lg(m);
    >
    > ++var;
    >
    > }
    >
    > t.join();
    >
    > //...
    >
    > }
    >
    >
    >
    >
    >
    > --- news://freenews.netfront.net/ - complaints: ---


    Thank you.
    -- Saeed
     
    Saeed Amrollahi, Dec 13, 2012
    #7
  8. On Thursday, December 13, 2012 12:13:29 PM UTC+3:30, David Brown wrote:
    > On 12/12/2012 19:37, Saeed Amrollahi wrote:
    >
    > > Sorry. I haven't read the manual yet. But my assumption simply

    >
    > > is: if there are no Valgrind warning messages, there is no

    >
    > > race condition problem!

    >
    > > -- Saeed

    >
    > >

    >
    >
    >
    > That's a very bad assumption. To avoid race conditions, you need to
    >
    > understand how threading works and how your locking and synchronisation
    >
    > work, and design your software so that it has no races. A tool like
    >
    > Valgrind can help catch mistakes, but it is only an aid - it will have
    >
    > false positives (cases when you know there is no race condition, but the
    >
    > tool can't prove it), and false negatives (cases where there /are/
    >
    > races, but the tool can't spot them).
    >
    >

    Thank you. May be It was better to read the manual in more detail
    at first place. I will do it certainly.
    From what you and Victor said, may be I misunderstood the
    capabilities and restrictions of the tool.
    About my assumption, please note, I just mean about the
    interface of Valgrind. Before using Valgrind,
    I check a lot my programs against data races, critical sections,
    and thread-safe concepts. Currently, I work and read very hard to understand
    concurrent programming and multithreading in C++11.
    >
    > And you /do/ need to read the manual and tutorials to get the best out
    >
    > of a tool like this.
    >
    >

    Sure.
    >
    > Correct multithreaded programming is not easy. It is easy to get it
    >
    > /mostly/ right - and easy to get something that works when you test it
    >
    > (race conditions seldom trigger during testing). But getting something
    >
    > that you are /sure/ is correct is a lot harder - and it starts with a
    >
    > full understanding of the topic, not with blind faith in magic tools.


    OK. May be after pulling out mutex to global space
    my simple program is OK, but I mistakenly thought it has data races.

    Regards,
    -- Saeed
     
    Saeed Amrollahi, Dec 13, 2012
    #8
  9. Saeed Amrollahi

    SG Guest

    Am 12.12.2012 22:01, schrieb Luca Risolia:
    > [...]
    > Anyway, if really want to protect shared data with *one* mutex, wrap it
    > with a std::lock_guard or a std::unique_lock instead of using the mutex
    > directly, as others have said. A std::lock_guard is faster and can be
    > used in a local {} block in your main() as well:


    What do you mean by "faster" in this context?! Faster in comparison to
    what and why?
     
    SG, Dec 13, 2012
    #9
  10. On Thursday, December 13, 2012 12:52:08 PM UTC+3:30, SG wrote:
    > Am 12.12.2012 22:01, schrieb Luca Risolia:
    >
    > > [...]

    >
    > > Anyway, if really want to protect shared data with *one* mutex, wrap it

    >
    > > with a std::lock_guard or a std::unique_lock instead of using the mutex

    >
    > > directly, as others have said. A std::lock_guard is faster and can be

    >
    > > used in a local {} block in your main() as well:

    >
    >
    >
    > What do you mean by "faster" in this context?! Faster in comparison to
    >
    > what and why?

    It's my question too. I read it before lock_guard is faster than
    unique_lock. I don't know why?
    -- Saeed
     
    Saeed Amrollahi, Dec 13, 2012
    #10
  11. Make an atomic increment; then you can forget the whole locking ^^:

    static std::atomic<int> aiVar;

    void incr()
    {
    aiVar.fetch_add( 1 );
    }

    int main()
    {
    aiVar.fetch_add( 1 );

    return 0;
    }
     
    Gerald Breuer, Dec 13, 2012
    #11
  12. Am 13.12.2012 15:44, schrieb Gerald Breuer:

    > Make an atomic increment; then you can forget the whole locking ^^:
    >
    > static std::atomic<int> aiVar;
    >
    > void incr()
    > {
    > aiVar.fetch_add( 1 );
    > }
    >
    > int main()
    > {

    std::thread thrIncr( incr );

    > aiVar.fetch_add( 1 );
    >
    > return 0;
    > }
     
    Gerald Breuer, Dec 13, 2012
    #12
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. The Weiss Family

    race conditions/pulse width

    The Weiss Family, Oct 16, 2004, in forum: VHDL
    Replies:
    6
    Views:
    756
    Jim Lewis
    Oct 19, 2004
  2. Taras_96
    Replies:
    7
    Views:
    6,841
    Taras_96
    Apr 5, 2005
  3. crazyrdx

    avoiding race

    crazyrdx, Jan 16, 2006, in forum: VHDL
    Replies:
    3
    Views:
    521
  4. mars
    Replies:
    6
    Views:
    377
    Laurent Pointal
    Feb 7, 2007
  5. Saxo
    Replies:
    37
    Views:
    838
Loading...

Share This Page