data race in multithreading code

[user123random]

For https://gist.github.com/promach/9d185d35a6e6db0da10992a19c36f754#file-thread-cpp , why am I facing so much data race issue at https://paste.ubuntu.com/p/SkzTY7ps5n/ ?

Do you guys have any general idea regarding the mutex that I have used on tologic ?

[coder777]

The mutexes are supposed to be on the same level as the variables to protect.
So why are tologic/fromlogic are global variables while the protectors tologic_mutex/fromlogic_mutex are not?

Instead of calling lock/unlock directly consider using lock_guard:

http://www.cplusplus.com/reference/mutex/lock_guard/

This makes the lock exception proof.

[user123random]

@coder777

It seems to me that lock_guard is used on the basis of exception throw.

I have no relevant std::logic_error or other exceptions to throw ...

Do you get what I just said above ?
Or am I missing some exceptions that I can throw regarding write_into_tologic() ?

Someone told me to use scoped_lock over lock_guard in new C++17 code.

https://en.cppreference.com/w/cpp/thread/scoped_lock

What do you guys think ?

[coder777]

lock_guard is a convenient way to lock/unlock with additionally exception safety. That's just a side note.

More important is the relation of ..logic and ...logic_mutex.

[user123random]

using lock_guard, unique_lock or scoped_lock for write_into_tologic() and write_into_fromlogic() does not help.

The data race comes from else where in the code.

Could you advise about this ?

[user123random]

Why do I have segmentation fault at line 131 of https://paste.ubuntu.com/p/6Wrzm6TQqH/ ?

did I call my logic_array constructor function correctly at line 235, 338 and 396 correctly ?

[coder777]

Ah i see. You learned how to protect the variables. Are those race conditions are gone?

did I call my logic_array constructor function correctly at line 235, 338 and 396 correctly ?

No. You have allocated the memory only locally. It does not longer exists after the constructor is done.

I suggest this:

class logic_array
{
	vector<RGB_packet> tologic;
	vector<YUV_packet> fromlogic;

	mutex tologic_mutex;
	mutex fromlogic_mutex;

public:
	logic_array(unsigned int image_width, unsigned int image_height) // constructor
	  : tologic(image_width * image_height), fromlogic_mutex(image_width * image_height)
	{
	}
...

And this means you don't need any global variables anymore.

Note that currently the logic_array is not shared across the threads. You should as before instantiate the logic_array in main() and pass it to the threads. Either as pointer or as reference.

[user123random]

@coder777

Someone helped me to fix the local memory issue that you raised.

See https://paste.ubuntu.com/p/rqtvxmspF5/

However, I am still getting the same amount of data race. Why ?

[coder777]

Someone helped me to fix the local memory issue that you raised.

Using all this global variables seems to be a step backwards.

Taking a look at the race conditions you see strlen, memcpy, etc. So it has nothing to do with the logic stuff. It looks like it is the mix of c functions (like printf(...)) and C++ classes like thread.