The usual pattern of using std::shared_timed_mutex is to let the 'reader' thread acquire it in shared mode and the 'writer' thread acquires it in exclusive mode. In this manner, the reads and writes cannot happen at the same time and thus the program is free from data-race/undefined behavior.

I wanted to understand if at all there's any problem if I change the mode among the threads i.e. the reader thread reads the shared variable after acquiring the lock in exclusive mode and the writer thread writes in the shared variable after taking the mutex in shared mode.

#include <iostream>
#include <thread>
#include <random>
#include <chrono>
#include <shared_mutex>

using namespace std::chrono_literals;

std::shared_timed_mutex lck;
int shared_array[5];

void writerFunc(int index);
void readerFunc();

//main thread
int main() {
  std::thread writer_threads[5];
  for(int i=0; i<5;   i) {
    writer_threads[i] = std::thread(writerFunc,i);
  }

  while(true) {
    std::this_thread::sleep_for(5s);
    readerFunc();
  }


  for(int i=0; i<5;   i) {
    writer_threads[i].join();
  }

}

//function executed in writer threads.
//Each writer thread will work on it's own index in the global shared array.
void writerFunc(int index) {
  std::random_device rd;
  std::mt19937 mt(rd());
  std::uniform_real_distribution<double> dist(1.0, 42.0);

  while(true) {
    {
      std::shared_lock<std::shared_timed_mutex> sl(lck);

      //Writing random number in shared variable.
      shared_array[index]  = dist(mt);
    }

    std::this_thread::sleep_for(100ms);
  }
}

//function executed in reader thread(main).
void readerFunc() {
  std::lock_guard<std::shared_timed_mutex> sl(lck);
  for(int i=0; i<5 ;   i) {
    std::cout<<"\nshared_array["<<i<<"]--> "<<shared_array[i];
  }
  std::cout<<"\n\n";
}

Since the reader and writer thread cannot concurrently access the variable at the same time, therefore, there's no data race in the above program. Thread-sanitiser also does not report any problem with the above program.

I'm mainly having a little doubt regarding the values read by the reader thread.

Is it guaranteed by the C standard, irrespective of the underlying CPU architecture, that

a) the above program doesn't have any UB?

b) the reader thread can only see the latest value written by the writer thread?

CodePudding user response：

unlock_shared explicitly synchronizes with subsequent lock calls on the same mutex. This would allow the reader to read data written by any of the writers. Similarly, lock_shared synchronizes with prior calls to unlock. So it is possible to use a shared_mutex backwards without a data race (note: rand is not required to be thread-safe).

But... should you?

The purpose of a mutex is to ensure data integrity, not just at the level of bytes (ie: data races), but at a higher level. You have 5 threads writing to 5 different locations. But... what is the meaning of the data? Are these data completely distinct from one another, or does the collection of data have some meaning that needs to be preserved? That is, if one thread writes to one value, does the reader get malformed information if another thread hasn't written its value yet?

If these data values are all completely, fundamentally separate, then a mutex is unnecessary (at least for basic types). What you're really doing is just atomic writes. The writers can write to an atomic<T>, and the reader will read these. Since the values are all disparate and lack any question of ordering between them, you don't need to block any thread from writing. All you need to do is ensure data integrity at the level of an individual T. Lock-free atomics will be much faster than any mutex-based solution.

But if the data has some notion of integrity to it, if the group of threads is collectively creating a single value that the reader thread should read in its entirety, then what you're looking for is a barrier, not a mutex. This object allows you to see if a group of execution agents have collectively reached a certain point. And if they have, it is safe for you to read the data. And once you're finished reading it, it is safe to release the agents to write to them once again.

CodePudding user response：

But Why?

I am wondering what the motivation behind changing the roles of the reader and the writer with respect to locking may be! What problem are you solving by doing so?

In a previous comment, you mentioned that you do not want contention between writers.

Looking at the code, I infer also that the update of every int in the array is independent of the others but the reader MUST see them all at once as if collectively they have ONE meaning (the reason for exclusive lock). You have not mentioned this yet - so assuming this is not the intention.

There is just one reader but many writers i.e. it looks inverted to a (some?) stereotypical case of having more readers than writers. This shouldn't be a major consideration.

Conveying an unintended meaning and surprising code should be avoided. I agree with @Nicol Bolas & suggest another approach too:

Wrong Tool - use std::atomic instead

The inverted use of std::shared_timed_mutex is a surprise here for the future maintainer (yourself?). Plus, using it is the source of misleading message to the reader and the reason for this question. I agree with @Nicol Bolas that atomic would solve this problem:

std::atomic<int> shared_array[5];

void writerFunc(int index) {
   ///Other code
    while(true) {
        //Writing random number in shared variable.
        shared_array[index].fetch_add(dist(mt));

        std::this_thread::sleep_for(100ms);
    }
}

void readerFunc() {
    for (auto& item : shared_array) {
        std::cout << item;
    }
}

Better Abstraction - use libguarded::shared_guarded

The root of the sorrow looks to be the level at which you have applied the std::shared_timed_mutex lck - it controls the entire array whereas you wish to have a finer control on every element.

I highly recommend you consider using the shared_guarded of the cs_libguarded available under BSD 2-Clause "Simplified" License.

libguarded::shared_guarded<int> shared_array[5];  //Nailed it!

void writeFunc(int index) {
    //Other code
    while (true) {
        {
            auto handle = shared_array[index].lock();
            auto& item = *handle;
            item  = dist(mt);
        }
        std::this_thread::sleep_for(100ms);
    }
}

void readerFunc() {
    for (auto& array_element : shared_array) {
        auto item = array_element.lock_shared();
        std::cout << *item;
    }
}

The above not only ensures the correct, unsurprising use of the shared data but also ensures const correctness as it does not allow writes on lock_shared. This can work with any data-types, not just ints - a restriction that std::atomic has. As @Solomon Slow points out, the memory barriers may cause unintended results with out-of-order execution with your original approach - this code doesn't have that problem. libguarded also ensures that access to shared data is always with the correct synchronization - no accidental use of shared data.

FYI, shared_guarded is equivalent to using a mutex for every element (as below), only much cleaner, const correct and fool-proof.

std::shared_timed_mutex lck[5];  //Don't do it by hand, better use libguarded, as above
int shared_array[5];

I strongly recommend prioritizing cleaner implementation over arbitrary objectives like not wanting to have many mutexes. If you do not want contention please eliminate sharing instead of aiming to reduce mutexes. The problem is sharing and not the existence of mutex.

P.S.: You tagged the question as C 14 while libguarded needs C 17. As far as I checked, libguarded::shared_guarded should work with std::shared_timed_mutex.