4
\$\begingroup\$

Stroustrup states [CPL 4, pg 118]:

The correspondance between the shared data and a mutex is conventional: the programmer simply has to know which mutex is supposed to correspond to which data. Obviously, this is error-prone, and equally obviously we try to make the correspondance clear through various language means.


It is better to make the correspondance between the shared data and a mutex explicit rather than implicit, for greater understandability.

Accordingly, I have designed a class to make this correspondance explicit.

In order to name this class, I have used the same convention used by the std::packaged_task class: it packages together a promise and a future and is therefore so called. Accordingly, I have named my class template packaged_mutex<T>, where T is the type of the variable being protected.


There are two possible design approaches for the packaged_mutex<T> class template:
1) As a wrapper for the std::mutex class
2) Extend the std::mutex class

The first approach has the following drawbacks:
1) The members of std::mutex are not directly accessible. I would have to provide an interface for these members.
2) I can't pass a packaged_mutex<T> directly to an std::lock_guard or an std::unique_lock. Only the wrapped std::mutex can be passed to these standard locks.

Both problems are automatically solved by using the second approach. Therefore, the packaged_mutex<T> class template derives publicly from std::mutex and thus is an std::mutex.

Please note that the objective of the packaged_mutex<T> class template is ONLY to explicitly associate an std::mutex with the variable being protected.

The objective is NOT to try out some other method of protecting a variable from concurrent access, such as Atomics.

I have also tried out a variant of the packaged_mutex<T> class template which tries to acquire a lock on the mutex, within the class itself, but it is currently giving compilation errors. Therefore, I am not submitting that version for code-review. The current version leaves locking to the client.


The packaged_mutex<T> class template is presented below:

/** packaged_mutex.h

    The packaged_mutex<T> class has the following basic functionality:
    1) It inherits the mutex class and is therefore a mutex.
    2) It includes a variable of type T. 
       This makes the association between the variable and the mutex explicit.

**/

#ifndef PACKAGED_MUTEX_BASIC
#define PACKAGED_MUTEX_BASIC

#include <mutex>        /// mutex

/// declarations ...


/// implementation ...

template<typename T>
class packaged_mutex : public std::mutex
{
private:
   T& var;

public:
   packaged_mutex(T& v) :
      var {v}
   {
   }

   void setvar(const T& val)
   {
      var = val;   
   }

   T& getvar()
   {
      return var;
   }
};  // packaged_mutex

#endif  ///  PACKAGED_MUTEX_BASIC

Note:
1) The constructor accepts an lvalue reference to the variable that the mutex is intended to protect. This reference is stored within the class. Thus, the association between the data to be protected and the mutex is made explicit.
2) packaged_mutex<T> provides a member setvar to set the value of the protected variable and a member getvar to retrieve its value.


The following is sample client-code:

/** Test the packaged_mutex<T> class (basic).
**/

#include <thread>           /// thread
#include <mutex>            /// lock_guard
#include <iostream>         /// cout, endl

#include "packaged_mutex.h"     /// packaged_mutex<T>

using namespace std;

using pckmtx_char = packaged_mutex<char>;

/// variables ...
char c {};
pckmtx_char pmtx {c};

/// declarations ...

void thread1();
void thread2();


int main()
{
   thread t1 {thread1};
   thread t2 {thread2};

   t1.join();
   t2.join();
}


void thread1()
{
   // critical section
   {
      lock_guard<pckmtx_char> lg {pmtx};
      pmtx.setvar('c');
   }  // release lock
}


void thread2()
{
   // critical section
   {
      lock_guard<pckmtx_char> lg {pmtx};

      cout << "variable has value " << pmtx.getvar()
           << endl;
   }  // release lock
}


Note:
1) Here, we create a packaged_mutex<char> (variable pmtx) which associates a char c with the packaged_mutex<char>:

    using pckmtx_char = packaged_mutex<char>;

    /// variables ...
    char c {};
    pckmtx_char pmtx {c};


2) The program starts two threads t1 and t2.

3) thread t1 contains a lock_guard which acquires the packaged_mutex<char>:

    lock_guard<pckmtx_char> lg {pmtx};


4) thread t1 then sets the value of the protected variable:

    pmtx.setvar('c');


5) thread t2 also contains a lock_guard which acquires the packaged_mutex<char>. It then accesses the protected variable and prints its value:

    cout << "variable has value " << pmtx.getvar()
         << endl;


6) As expected, the packaged_mutex serializes access to the protected variable and thus avoids a data race.

Comments and suggestions are requested.

\$\endgroup\$
  • \$\begingroup\$ @bipll, I don't see a problem there. (In fact the program works, as the links I posted demonstrate.) The val argument is declared const so that it might not be modified within the setvar method. However, the private variable var can be modified by the setvar method. There's no contradiction there. \$\endgroup\$ – SSteven May 17 '18 at 11:57
  • \$\begingroup\$ Sorry, I somehow thought for a moment you're rebinding a reference rather than assigning a value to the underlying object. >_< This method should probably be templated and perfect-forwarding BTW. \$\endgroup\$ – bipll May 17 '18 at 12:04
  • \$\begingroup\$ @bipll, This method should probably be templated and perfect-forwarding. Can you clarify what you mean, please? Thanks. \$\endgroup\$ – SSteven May 17 '18 at 12:09
  • \$\begingroup\$ See my edited answer. \$\endgroup\$ – bipll May 17 '18 at 12:12
5
\$\begingroup\$

I don't have time to give this the comprehensive review that it deserves, but there are a few things worth mentioning.

Mutexes are perhaps most often used to ensure that the abstract state represented by two or more variables together doesn't get out of sync: for example they might have to protect an array and the index in the array that next needs writing to. It's worth considering how this should interact with a single private variable. Yes, wrapping the variables in question together into a struct or class is a viable answer to this within the bounds of your solution, but it wants considering even so.

Standard mutexes are generally quite slow compared to other synchronisation primitives, and plausibly a poor choice if you are doing simple operations on them. This may be less of a concern if you're doing heavier computations than in your example.

Your testing is not sufficient to demonstrate that the solution works as intended. Aside from timing, it would not behave differently from if you didn't bother to lock the mutex! At a minimum, ensure for testing that:

  1. You're running plenty of examples, to maximise the chance of getting a collision.
  2. Each thread is both reading from and writing to the shared state so that interference is detectable.

Little stuff:

It would probably be more idiomatic to use get rather than getvar. This would match, say, how the std::tuple works.

Generally avoid using namespace std; because it pollutes your files with more standard symbols than can be reasonably kept track of. This is less bad if you at least avoid it in headers, but as a matter of habit it's best to just always use std:: explicitly.

There is no real need to be stingy about variable names. Dropping vowels saves you a smidge of typing time and costs you clear thinking time, which is the much more valuable resource!

\$\endgroup\$
  • \$\begingroup\$ thanks for your review. (1) For multiple variables, I was thinking of using a variadic ctor. (2) In this version, I have left locking to the client, for reasons I stated in my post. (That version currently gives compilation errors.) (3) I am already using std:: in the header. I opened std:: only in the client-code. \$\endgroup\$ – SSteven May 17 '18 at 7:37
  • 1
    \$\begingroup\$ Multivariable version can possibly be easily implemented if you partly specialize the class for tuple of references. \$\endgroup\$ – bipll May 17 '18 at 10:59
  • \$\begingroup\$ @Josiah, I have implemented some of your suggestions. \$\endgroup\$ – SSteven May 19 '18 at 16:27
1
\$\begingroup\$

As a side note, you only package a std::mutex. What about shared_mutex? Other mutex types? Perhaps, it makes sense to make parent class a template parameter.

setvar can possibly be generalized as

template<typename U> packaged_mutex &setvar(U &&that) {
     var = std::forward<U>(that);
     return *this;
};
\$\endgroup\$
  • \$\begingroup\$ Excellent point. Definitely. \$\endgroup\$ – SSteven May 17 '18 at 12:00
  • \$\begingroup\$ I have implemented some of your suggestions. \$\endgroup\$ – SSteven May 19 '18 at 16:28
0
\$\begingroup\$

Thanks for the review points. I have now implemented the following suggestions.

1) First, there was a suggestion to parameterize the inherited mutex, so that the class could inherit from any mutex type: mutex, recursive_mutex, timed_mutex and timed_recursive_mutex.

Accordingly, I have named the new class template packaged_Mutex:

template<typename Mutex,
         typename Var>
class packaged_Mutex : public Mutex
{
/// ...
}

Here, Mutex is the type of the mutex being inherited and Var the type of the variable being packaged.


2) As suggested, I have made the testing a bit more robust. There are 3 test-cases:
a) packaged_Mutex<mutex, int>
b) packaged_Mutex<recursive_mutex, char>
c) a functional test that includes setting and getting the packaged variable from 2 asynchronous threads.


3) There was a suggestion to rename the setvar() member to set() and the getvar() member to get(). However, keep in mind that a packaged_Mutex is a Mutex by inheritance. A name such as set() might suggest that it is the Mutex that was being set, rather than the packaged variable. Therefore, to make the distinction clear, I have retained the old nomenclature.


4) The specialization for a Var type of std::tuple has been deferred to a later version. This would require run-time iteration of a variadic, which is not simple.


5) I have also fixed the previous compilation error caused by trying to lock, from within the class itself.


Here's the packaged_Mutex class template:

/** packaged_Mutex.h

    The packaged_Mutex<Mutex, Var> class has the following functionality:
    1) It inherits type Mutex which can be any mutex class and is therefore a Mutex.
    2) It includes a variable of type Var, which is the variable to be protected
       during concurrent access. 
       The inclusion makes the association between the variable and the mutex explicit.
    3) It includes member setvar() to set the variable's value and 
       member getvar() to get the variable's value.

**/

#ifndef PACKAGED_MUTEX_ADVANCED
#define PACKAGED_MUTEX_ADVANCED

#include <mutex>        /// mutex, lock_quard<>


/// declarations ...
/// classes ...

template<typename Mutex,
         typename Var>
class packaged_Mutex : public Mutex
{
private:
   Var& var {};

public:
    packaged_Mutex(Var& v);
    void setvar(const Var& val);
    Var& getvar();
};  // packaged_Mutex


/// implementation ...
/// packaged_Mutex<Mutex, Var> members ...

template<typename Mutex,
         typename Var>
packaged_Mutex<Mutex, Var>::packaged_Mutex(Var& v) :
   var {v}
{
}


template<typename Mutex,
         typename Var>
void packaged_Mutex<Mutex, Var>::setvar(const Var& val)
{
   std::lock_guard<packaged_Mutex> lg {*this};

   var = val;
}


template<typename Mutex,
         typename Var>
Var& packaged_Mutex<Mutex, Var>::getvar()
{
   std::lock_guard<packaged_Mutex> lg {*this};

   return var;
}

#endif  ///  PACKAGED_MUTEX_ADVANCED


And here's the test-case:

int main()
{
   test_mutex();
   test_recursive_mutex();
   functional_test();
}


void test_mutex()
{
   cout << "test_mutex() ... " << endl;

   int i;
   packaged_Mutex<mutex, int> pmi {i};

   pmi.setvar(3073);

   cout << "pmi.getvar() = " << pmi.getvar()
        << endl << endl;
}


void test_recursive_mutex()
{
   cout << "test_recursive_mutex() ... " 
        << endl;

   char c;
   packaged_Mutex<recursive_mutex, char> prmc {c};

   prmc.setvar('A');

   cout << "prmc.getvar() = " << prmc.getvar()
        << endl << endl;
}


void functional_test()
{
   cout << "functional_test() ... " 
        << endl;

   int i;

   packaged_Mutex<mutex, int> pmi {i};

   // Launch a thread to write to the protected variable.
   thread wr {writer, ref(pmi)};

   // Launch a thread to read the protected variable.
   thread rd {reader, ref(pmi)};

   wr.join();
   rd.join();
}


void writer(packaged_Mutex<mutex, int>& pm)
{
   pm.setvar(4901);

   /// Signal that the "wr" thread has prepared the condition.
   {
      lock_guard<mutex> lg {mtx};

      rdy = true;
   }  // release lock

   /// Signal the "rd" thread it can continue.
   cnd.notify_one();
}


void reader(packaged_Mutex<mutex, int>& pm)
{
   // Wait until the "wr" thread signals "ready".
   {
      unique_lock<mutex> ul {mtx};

      cnd.wait(ul, [] {return rdy;});
   }  // release lock

   cout << "protected variable = "
        << pm.getvar()
        << endl;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.