Should I use lock_guard, scoped_lock or unique_lock in this situation?

Question

I have read many of the questions already answered that relate to this but none of them gave me a clear understanding of which I should be using when I have multiple writers but a single reader. The code below is a contrived example of what I'm talking about.

struct StateInfo {
    long wd{};
    uint32_t perc{};
};

class Blah 
{
    const int numDevices = getDevices();
    std::shared_mutex sharedMutexSI_;
    vector<StateInfo> stateInfo;
public:

    Blah() : stateInfo(numDevices){};

    void writers(StateInfo &newSi, const int i)
    {
        std::shared_lock<std::shared_mutex> _MULTIPLE(sharedMutexSI_);
        stateInfo[i] = newSi;
    }

    StateInfo reader(const int i)
    {
        std::lock_guard<std::shared_mutex> _EXCLUSIVE(sharedMutexSI_);
        return stateInfo[i];
    }
};

The situation is that multiple writers may simultaneously update the stateInfo vector but never the same item in the vector as i is unique to each thread. A single reader thread may at any time try to read any of the vector items.

1. Is the above code correct in avoiding race conditions?

2. Is lock_guard the right one to use or should I use scoped_lock or unique_lock?

Restive

There is no difference between lock_guard, scoped_lock and unique_lock in this use case – Metal

Depending on the size of long on your platform you could change StateInfo to std::atomic<uint64_t> and avoid locking all together – Metal

@AlanBirtles so lock_guard is the right one as it is the simplest and the others don't add anything useful in this case? – Restive

Seems in 17 lock_guard is unfavoured or supersetted by scoped_lock? – Sarcenet

Both, lock_guard and scoped_lock are fine, unique_lock is an overkill. – Tolentino

My understanding is that lock_guard is the simplest and if I don't need the functionality of scoped_lock, then I should stick with the simplest? – Restive

At least in libstdc++, for a single mutex lock_guard and scoped_lock are identical in their implementation. – Tolentino

If (number of readers) times (number of writers) to X is positive (i.e. > 0) all readers and writers of X must be synchronised. Then it depends on how many synchronisation primitives (e.g. mutex) is acceptable. With exactly one primitive, all readers and writers must use it (so all writers synchronise with each other and the reader). If n mutexes (n = number of elements) is acceptable, then each writer can use its own primitive (and not synchronise with other writers) but the reader must grab the corresponding primitive before reading each element (i.e. synchronise with all writers). – Kola

Regarding lock_guard, scoped_lock and unique_lock: see a dedicated question with answers here. – Inae

You use shared_lock for writing and scope_lock for reading? That is opposite of what you want. Use shared_lock for reading and scope_lock for writing. – Benjaminbenji

@ALX23z, OP has one reader and several writers, and there are no data races between writers. – Tolentino

@Tolentino you mean he has writers dedicated to each index? This is a presumption made outside of class which isn't good in general. In this case it might be preferable to have each state info have its own std::mutex or just make the StateInfo atomic. In any case the class suffers from false-sharing so I am not sure if shared lock helps much. – Benjaminbenji

@ALX23z, yeah, that's what Alan Birtles and Peter suggested above. – Tolentino

Answer 1

To summarize what was already written in comments:

Yes, the code is correct. However, it may be inefficient, because it disallows reading from any array element while writing to another array element. You might want to do more fine-grained synchronization by using a mutex for each array element.

class Blah 
{
    const int numDevices = getDevices();
    std::vector<std::mutex> mutexes;
    std::vector<StateInfo> stateInfo;
public:

    Blah() : mutexes(numDevices), stateInfo(numDevices){}

    void writers(StateInfo &newSi, const int i)
    {
        std::lock_guard<std::mutex> guard(mutexes[i]);
        stateInfo[i] = newSi;
    }

    StateInfo reader(const int i)
    {
        std::lock_guard<std::mutex> guard(mutexes[i]);
        return stateInfo[i];
    }
};

Inae