3

I need to work with the same mutex and unique_lock across the main function and class instances. However, I am having trouble assigning the mutex/unique_lock address to a class member variable (that is a mutex&).

This is what I have:

Worker.h

class Worker 
{
private:

    std::mutex &m_mu;
    std::unique_lock<std::mutex> &locker;

public:

    void start(std::mutex &mu, std::unique_lock<std::mutex> &locker);

};

Worker.cpp

void Worker::start(std::mutex &mu, std::unique_lock<std::mutex> &locker)
{
    this->mu = mu; // error 
    this->locker = locker; // error
}

I tried doing this->mu(mu); but that doesn't work either. Is there anything I can do to make this work?

Thanks.

Matthieu Brucher
  • 21,634
  • 7
  • 38
  • 62
Andrej
  • 35
  • 1
  • 4
  • 1
    There is no way to change *what* a reference refers to. `this->m_mu = mu;` is trying to copy the "value" of `mu` and assigning that "value" to whatever `this->m_mu` is currently referring to. – François Andrieux Oct 17 '18 at 15:43
  • 1
    You do not need both mutex and the unique_lock as members of your class, this is redundant. – SergeyA Oct 17 '18 at 15:47
  • This smells bad. Most of the time a single class should be taking care of making itself threadsafe. I don't know why main would own a mutex or lock. – Christopher Pisz Oct 17 '18 at 15:59
  • 2
    @ChristopherPisz this is not true. There are multitude of very reasonable cases when a single class should not make itself thread-safe. – SergeyA Oct 17 '18 at 16:03
  • 1
    @SergeyA I'll fed-ex you $5 if you show a listing where it makes sense to have main own a mutex and pass it around to other classes, rather than having any shared resource wrapped in a class that manages itself, while using proper OO, and C++ – Christopher Pisz Oct 17 '18 at 16:10
  • @ChristopherPisz you are not thinking out of the box. Simplest example will be when mutex is used a sort of gate - for example, to throttle some operation. I will not write code for this, so you can keep your $5. – SergeyA Oct 17 '18 at 16:19
  • @Sergey I have never seen this particular box in 35 years. I've seen people who wanted to make the box, but I offered them a steel chest instead. If you can't provide an example here or in private chat, then I fear all our words are wasted and noone will learn anything. – Christopher Pisz Oct 17 '18 at 16:39
  • @Andrej you should not be sharing a `unique_lock` object between threads. That type is not thread-safe and not meant for concurrent access. The `mutex` is what you share, and each execution agent that wants to synchronise on that mutex creates its own `unique_lock` object to acquire and release the lock on the mutex. – Jonathan Wakely Oct 17 '18 at 18:54
  • @ChristopherPisz _"Most of the time a single class should be taking care of making itself threadsafe."_ [...] _"using proper OO, and C++"_ But what if you're not using an OO design? There's more than one way to design software. – Jonathan Wakely Oct 17 '18 at 18:59
  • @Jonathan In that case, we'd be talking about 'C' rather than 'C++', even if we opted to have that C use some C++ features. It's fairly accepted that C++, the language and the mindset, is object orientated, and embraces RAII. Neither 'C', the language, nor the mindset (which is being alluded to here) is object orientated. – Christopher Pisz Oct 17 '18 at 19:23
  • @ChristopherPisz that's utter nonsense! It's perfectly possible (and very popular) to write non-OO C++, while still using C++ features, not just using C. I think either you don't understand what OO means or you don't understand C++. RAII is not a feature of OO, for example. [Is C++ an Object-Oriented Language?](http://stroustrup.com/bs_faq.html#Object-Oriented-language) and [What is "multiparadigm programming"?](http://stroustrup.com/bs_faq.html#multiparadigm) and [Why C++ isn't just an Object-Oriented Programming Language](http://stroustrup.com/oopsla.pdf) go into more detail. – Jonathan Wakely Oct 17 '18 at 19:34
  • Strousup is right that the language itself does not force you to write OO code. He also said you are free to blow your leg off. However, good practice, the C++ job market, and maintainability do have such requirements, whether you like them or not. Show me your mutex from main, non RAII, non OO code, and I'll show you bugtracker over 35 years of C++. Also, you can call passing a mutex from main to various classes, whatever you like. I'll call it unmaintainable, error prone, and undoubtedly the wrong way to do things, until shown a listing that proves otherwise. – Christopher Pisz Oct 17 '18 at 21:12
  • Bottom line is that the mutex should live with the resource it protects. – Christopher Pisz Oct 17 '18 at 21:12

2 Answers2

6

You need to pass the mutex reference when you construct your class.

Worker::Worker(std::mutex &mu, std::unique_lock<std::mutex> &locker)
:m_mu(mu), locker(locker)
{}

That's the only place you can initialize a reference. Once it's constructed, you cannot change what it references.

Why do you need the locker? The mutex makes the synchronization, the lock is just a RAII object to ease acquiring the mutex.

Matthieu Brucher
  • 21,634
  • 7
  • 38
  • 62
4

You don't need to pass the lock object to the function. As long as the class is referring to the correct mutex you can lock the mutex inside the function like this:

class Worker
{
private:
    std::mutex& m_mu;

public:
    Worker(std::mutex& mu): m_mu(mu) {} // bind reference during initialization

    void start();

};

// Worker.cpp

void Worker::start()
{
    std::unique_lock<std::mutex> locker(m_mu); // lock the shared resource

    // Do something with it here
}

int main()
{
    std::mutex mu;

    std::vector<Worker> workers(4, Worker(mu));

    // etc...
}
Galik
  • 47,303
  • 4
  • 80
  • 117
  • Thanks for the tip. Is this approach going to work across multiple threads (of the same class type)? Also, I am getting a compile error: Worker::Worker(const Worker &)': attempting to reference a deleted function ----- when trying to initialize an array of Worker type std::vector worker(threadsCountInt, Worker(std::ref(mu))); – Andrej Oct 17 '18 at 18:36
  • @AndrejTkáč That should be fine as long as they lock the **same** *mutex*. I am not getting the error you are getting with my exact code. Did you do something different? – Galik Oct 17 '18 at 18:48
  • The error means he's trying to copy a `Worker` object, which won't work because the reference makes it non-copyable. I think there's a bigger design problem with the whole program. – Jonathan Wakely Oct 17 '18 at 18:53
  • I guess we don't need to use `std::ref` in this case, right? (Unless `std::thread` is used.) – starriet Jul 11 '23 at 02:05
  • 1
    @starriet Yes, you're right, `std::ref` was completely unnecessary. Now fixed. – Galik Jul 11 '23 at 03:26