Sharing data with another thread using lock_guard

Question

I wrote a updater_thread() and a reader_thread(), and used lock_guard to protect a global instance.

Question

I've never used lock_guard. In this code, is lock_guard used properly?
Is this code thread-safe?

Code

#include <iostream>
#include <thread>
#include <mutex>
#include <iterator>
#include <algorithm>
#include <vector>
#include <initializer_list>
#include <unistd.h>

class C {
  std::vector<int> v;
  std::mutex mutex_;

public:
  C() {}
  void Update(const std::vector<int> &new_v) {
    std::lock_guard<std::mutex> lock(mutex_);
    v = new_v;
  }
  bool Check(const int x){
    std::lock_guard<std::mutex> lock(mutex_);
    return std::find(v.begin(), v.end(), x) != v.end();
  }
  /* dump() is not essential */
  void dump() {
    std::lock_guard<std::mutex> lock(mutex_);
    std::cout << "dump: ";
    std::copy(v.begin(), v.end(),
              std::ostream_iterator<int>(std::cout, " "));
    std::cout << "\n";
  }
};

// create an instance gloablly
C g_c;

void updater_thread() {
  std::cout << "start updater_thread\n";
  while (true) {
    std::vector<int> v;
    for (int i = 0; i < 10; i++) {
        v.push_back(rand() % 10 + 1);
    }
    std::sort(v.begin(), v.end());
    g_c.Update(v);
    std::cout << "updated!!!\n";
    std::copy(v.begin(), v.end(),
              std::ostream_iterator<int>(std::cout, " "));
    std::cout << "\n";
    sleep(5);
  }
}

void reader_thread() {
  std::vector<int> v {1,2,3,5};
  while (true) {
    std::cout << "check: non-exist item: ";
    for (int i = 0; i < v.size(); i++) {
        if (!g_c.Check(v[i])){
            std::cout <<  v[i] << " ";
        }
    }
    std::cout << "\n";
    sleep(1);
  }
}

int main() {
  std::thread t_up(updater_thread);
  std::thread t_r(reader_thread);
  t_up.join();
  t_r.join();
}

Quuxplusone · Accepted Answer · 2019-01-31 02:06:09Z

Yes, that all looks correct to me!

You don't use anything from <unistd.h> (non-standard) or <initializer_list> (obscure), so I recommend removing those includes.

You sigil the mutex_ data member with an underscore, but you don't sigil the v member. I recommend being consistent:

std::mutex mutex_;
std::vector<int> v_;

Personally I would spell the member's name mtx_, but that's just a personal habit; I don't know if that naming convention is widespread. (cv_ for a condition variable certainly is, though!)

Consider that your Check and dump methods don't need to mutate the object, so they should be declared const. This means that your mutex_ data member will need to be declared mutable so that you can still lock and unlock it inside your const member functions.

Also, consider picking a capitalization rule and sticking to it. Why Check and Update but dump (not Dump)?

Check's parameter x is marked const but that marking serves no purpose: eliminate it. (Const is a contract.)

for (int i = 0; i < v.size(); i++) {
    if (!g_c.Check(v[i])){
        std::cout <<  v[i] << " ";
    }
}

This could be rewritten more concisely as

for (int vi : v) {
    if (!g_c.Check(vi)) {
        std::cout << vi << " ";
    }
}

Your multithreading stuff all looks great!

Stack Exchange Network

Sharing data with another thread using lock_guard

Question

Code

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Sharing data with another thread using lock_guard

Question

Code

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions