-1

By design, class B is uncopiable but movable.

Recently, I enjoy that B has automatically-generated move constructor and move assignment operator.

Now, I add std::mutex to class B.
The problem is that std::mutex can't be moved.

#include <iostream>
#include <string>
#include <vector>
#include <mutex>
class C{
    public: C(){}
    public: C(C&& c2){ }
    public: C& operator=(C&& c2){ return *this; }
    //: by design, I don't allow copy, can only move
};
class B{public: int a=0;
    C c;
    //------ 10+ trivial fields ---
    //private: std::mutex mutexx;  //<=== uncomment = uncompilable
};
int main(){
   B b1;
   B b2;
   b1=std::move(b2);
}

Is there a solution to make it compile?

My poor workaround 1 (manual)

Manually create 2 move functions.

class B{public: int a=0;
    C c;
    //------ 10+ trivial fields ---
    private: std::mutex mutexx;
    public: B(B&& b2){
         this->c=std::move(b2.c); 
         //--- copy other trivial fields (long and hard to maintain) --
         //:: intentionally omit move/copy of mutex
    }
    public: B& operator=(B&& b2){ ... }
};

This workaround creates the effect that I want.

My poor workaround 2 (unique_ptr)

Use std::unique_ptr<std::mutex>, but there are 3 minor disadvantages :-

  1. Very ugly
  2. Add a bit of extra heap & cpu cost. The cost might be little, but I will resent it forever.
  3. I also need to refactor some of my code from mutexx.lock() to mutexx->lock()

Edit : My poor workaround 3 (encapsulate mutex)

(Add after get the first answer)

Encapsulate std::mutex into a new class, and create 2 move functions that do nothing.

If future reader may use this approach, beware that my question itself is questionable.
Thank "ALX23z" and "j6t" for warning.

cppBeginner
  • 1,114
  • 9
  • 27
  • 2
    There's a reason why `std::mutex` is not movable. Say, one thread locks it and another moves it. How should it behave? Trying to move an object with a mutex, or even with a dynamically allocated mutex (say, via `std::unique_ptr`) is an easy way to find yourself in various data-races and UB. Consider using thread safety libraries like `libguarded` or store classes with mutexes only via some smart pointers. – ALX23z Jul 18 '22 at 06:13
  • @ALX23z :: In my use case, I want to ignore such problem. (my poor workaround 1 implies that). If it ever happen, I am ok with undefined behavior. Thank, I will look into libguarded. ^^ – cppBeginner Jul 18 '22 at 06:16
  • 3
    The design is questionable. If the mutex protects something in the instances of `B`, `b1` and `b2`, how do you guarantee that the protection in `b1` is upheld at the time when `b2` is moved over to `b1`? The only reasonable solution is to have a move assignment operator in `B` that guarantees the protection protocol. From the design perspective, it does not make a lot of sense to depend on the compiler-generated move assingment operator. – j6t Jul 18 '22 at 06:16
  • 1
    Note that you don't have to keep repeating `public:`. An access declaration applies to every name that comes after it, until there's another access declaration. – Pete Becker Jul 18 '22 at 13:24
  • @Pete Becker :: You are right. Thank. It is my bad habit. – cppBeginner Jul 19 '22 at 00:25

1 Answers1

1

I enjoy that B has automatically-generated move constructor

Why don't rename class B and use it as a base class?

#include <iostream>
#include <string>
#include <vector>
#include <mutex>
class C{
    public: C(){}
    public: C(C&& c2){ }
    public: C& operator=(C&& c2){ return *this; }
    //: by design, I don't allow copy, can only move
};
class B1{public: int a=0;
    C c;
    //------ 10+ trivial fields ---
};

class B : public B1 {
    public: B& operator=(B&& b2){ return *this; }
    private: std::mutex mutexx;
};

int main(){
   B b1;
   B b2;
   b1=std::move(b2);
}
273K
  • 29,503
  • 10
  • 41
  • 64
  • This answer is useful. Thank. It is just my feeling, but I still wish that I don't need chunky feature like inheritance for this little thing. – cppBeginner Jul 18 '22 at 06:12
  • 1
    If you do not want it, you can place B1 in the inside `class B { B1 b1; public: B& operator=(B&& b2){ return *this; } private: std::mutex mutexx; };`. There is no other simpler ways. – 273K Jul 18 '22 at 06:14
  • 1
    BTW using `std::mutex` unique in each object is a bad practice. Consider using `static std::shared_mutex B::mutexx;` – 273K Jul 18 '22 at 06:16
  • :: thank, I never heard that such practice is a bad practice. ^^ – cppBeginner Jul 18 '22 at 06:18
  • 2
    I would not say it is bad practice. It all depends on what you want to achieve. – j6t Jul 18 '22 at 06:18
  • @j6t Your comment sounds strange taking into account your comment to the question. – 273K Jul 18 '22 at 06:20
  • 3
    @273K I'd say the opposite, using a static mutex is a bad practice unless you write a singleton. – ALX23z Jul 18 '22 at 06:23
  • 1
    @ALX23z Singleton is an [anti-pattern](https://stackoverflow.com/questions/12755539/why-is-singleton-considered-an-anti-pattern). – 273K Jul 18 '22 at 06:35
  • @273K decide, either using a singleton is bad practice or not. Cause static mutex is effectively a singleton. – ALX23z Jul 18 '22 at 06:57