Data Race problem

[Saeed Amrollahi]

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

 

[Victor Bazarov]

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

 

[Saeed Amrollahi]

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

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]

(e-mail address removed):









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

 

[Victor Bazarov]

[..] 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

 

[Saeed Amrollahi]

[..] 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

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]

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: (e-mail address removed) ---

Thank you.
-- Saeed

 

[Saeed Amrollahi]

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

 

[SG]

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?

 

[Saeed Amrollahi]

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

 

[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()
{
aiVar.fetch_add( 1 );

return 0;
}

 

[Gerald Breuer]

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 );