Add threads jobs into Thread array

Question

I attend to working RWLock with 5 thread ( 4 threads for reading and 1 thread for writing ), reference(&) operator and without using mutex library

I want to make a thread array and insert the their jobs, and working. but when i had a breakpoint to funtion ReadLock, i got a value 5. It means threads are inserted last value of threadNum. It didn't attend ( i think threads are working once)

My question is how to insert correctly their values and work correctly

code is below

#include <iostream>
#include <thread>
#include <windows.h>


void ReadLock(bool& try_w_lock, bool*& readFlag, int threadNum, int& Val);
void WriteLock(bool& try_w_lock, bool*& readFlag, int& Val);

int main()
{
    std::thread th[5];
    bool try_w_lock = false;
    bool Flag[5] = { false, };
    bool* readFlag = Flag;
    int Val = 0;
    int threadNum = 0;

    for ( ; threadNum < 4; ++threadNum)
    {
        th[threadNum].operator=( std::thread([&]() {ReadLock(try_w_lock, readFlag, threadNum, Val); }));
    }
    for ( ; threadNum < 5; ++threadNum)
    {
        th[threadNum].operator=(std::thread([&]() {WriteLock(try_w_lock, readFlag, Val); }));
    }


    for (auto& d : th)
    {
        d.join();
    }
}

void ReadLock(bool& try_w_lock, bool*& readFlag, int threadNum, int& Val)
{
    while (Val < 100)
    {
        while (try_w_lock);
        (readFlag)[threadNum] = true;
        
        std::cout << Val << std::endl;

        (readFlag)[threadNum] = false;
        Sleep(500);
    }
    
}

void WriteLock(bool& try_w_lock, bool*& readFlag, int& Val)
{
    while (Val <100)
    {
        try_w_lock = true;
        while (true)
        {
            bool b = false;
            for (int i = 0; i < 5; ++i)
            {
                b = b | (readFlag)[i];
            }
            if (!b)
                break;
            else;
        }
        ++Val;
        try_w_lock = false;
        Sleep(500);
    }
}

This code is quite remarkably unsafe. It would be a good idea to use [`mutex`](http://cplusplus.com/reference/mutex/mutex/?kw=mutex) and [`atomic`](http://cplusplus.com/reference/atomic/atomic/) types — IWonderWhatThisAPIDoes, Mar 03 '21 at 08:07
Side note: operators are designed to be called with their operator name, so you could simply write `th[threadNum] = std::thread([&]() { /*...*/ });`. — Aconcagua, Mar 03 '21 at 08:37
*' `mutex library`'* – do you mean entire [*thread support* library](https://en.cppreference.com/w/cpp/header) (I'd assume so) or only the mutex *header*? — Aconcagua, Mar 03 '21 at 09:35
You might want to consult your favourite search engine for lock free algorithm... — Aconcagua, Mar 03 '21 at 09:49

score 0 · Accepted Answer · answered Mar 03 '21 at 12:04

In this line:

th[threadNum] = std::thread([&](){ReadLock(try_w_lock, readFlag, threadNum, Val); });

you capture all lambda implicit parameters by reference. This includes threadNum. Thus, all your threads refer to the same location in memory that is being quickly changed in the for loop. By the time threadNum has been passed to ReadLock (or WriteLock), its value may have changed. This is called a race condition, see: What is a race condition?
This explains the behavior you experienced with your debugger.

If you cannot spot things like this, and do not know any tools that would help you in your debugging efforts (e.g. valgrind, TSAN), then I suppose you haven't got much experience in multithreading. If so, please do not waste your time trying to bypass atomics, mutexes or any other secure way of inter-thread synchronisation. Take a good, comprehensive book / course on multithreading first. It's not a thing that can be learned by try-and-error.

Thanks for your help. and Your advise is very helpful to me. In beginner step, i realized that making a thread algorithm needs lots of effort and knowledge.... accept your advice, i will take a course for understanding threads :) I want to say thank you again — Leezy, Mar 04 '21 at 00:24

Add threads jobs into Thread array

1 Answers1