1

I have the following code. The LockWrap is what I want to add into a std::map.

The LockWrap inherits a noncopyable which is exactly from the boost library. And I can't modify the definition of LockWrap.

#include <mutex>
#include <map>

struct base_token {};

class noncopyable: base_token
{
protected:
  constexpr noncopyable() = default;
  ~noncopyable() = default;
  noncopyable( const noncopyable& ) = delete;
  noncopyable& operator=( const noncopyable& ) = delete;
};

class LockWrap : private noncopyable
{
public:
    explicit LockWrap(std::mutex & mut)
        : lock_(mut)
    {}
    std::lock_guard<std::mutex> lock_;
};

LockWrap genLockWrap(std::mutex & mut) {
    return LockWrap(mut);
}

int main() {
    std::mutex mut;
    typedef std::map<int, LockWrap> map_type;
    map_type locks_held;
    int id = 1;
    
    // None of the following can help.
    // locks_held.emplace(std::move(1), genLockWrap(mut));
    // locks_held.insert(map_type::value_type{std::move(1), genLockWrap(mut)});
    // locks_held.insert(std::move(map_type::value_type{std::move(1), genLockWrap(mut)}));
}

What I want is to add LockWrap objects into locks_held. However, non of my attempt works. The std::map<>::insert using non-copyable objects and uniform initialization 's way not works either. I think it is because I don't have a move constructor.

I know I can create a uniformed lock on multiple mutex to work around my problem. However, I still want to know how to solve this problem.

calvin
  • 2,125
  • 2
  • 21
  • 38
  • The use of `genLockWrap` prevents use of the map, since even with `try_emplace` you need a move constructor (or copy constructor). Your options are making `LockWrap` movable by using `std::unique_lock` instead of `std::lock_guard` and removing the base class (the member makes the class movable, but not copyable anyways). Alternatively you could use `try_emplace` as described in the answer. – fabian May 26 '23 at 11:39
  • You can either construct the `LockWrap` object within the map directly (with `locks_held.emplace(1, mut);`): [godbolt example](https://godbolt.org/z/o8a6GTa8G) or make your `noncopyable` and `LockWrap` types moveable (requires the use of `std::unique_lock`): [godbolt example](https://godbolt.org/z/xG7fKTaaj). Note that it would probably be a better idea to use [`std::scoped_lock`](https://en.cppreference.com/w/cpp/thread/scoped_lock) when locking multiple mutexes, because that class comes with a built-in deadlock prevention. – Turtlefight May 26 '23 at 11:40
  • Dupe/related: [map/unordered_map with non-movable, default constructible value type](https://stackoverflow.com/questions/22229773/map-unordered-map-with-non-movable-default-constructible-value-type) – Jason May 26 '23 at 12:47

3 Answers3

2

The reason that your attempts fail, is that your LockWrap class is not only non-copyable, but also non-movable due to the lock_guard member. Therefore, using a creator-function which requires a move constuctor to move it into the map, will not work.

This you can neither copy, nor move the object into the map, and the remaming option is to create it directly in-place in the map.

try_emplace is your friend then:

locks_held.try_emplace(id, mut);

try_emplace creates the object in-place, so there is no copying involved. You simply pass the parameters needed for the constructor.

cptFracassa
  • 893
  • 4
  • 14
2

This is where you use the most useful type for constructing non-movable types:

// Also known as "lazy"
template<typename F>
struct construct_from {
    F f;
    constexpr operator decltype(f())() const&& {
        return f();
    }
};
template<typename F> construct_from(F) -> construct_from<F>;

Then you simply use:

locks_held.emplace(std::move(1), construct_from{[&]{ return genLockWrap(mut); });

This will construct the LockWrap member of the pair when it is converted so it doesn't need to be moved.

Artyer
  • 31,034
  • 3
  • 47
  • 75
1

You basically have 2 options:

  • Either you construct the LockWrap within the map (by just passing the mutex to locks_held.emplace(), e.g.:
    godbolt
    locks_held.emplace(1, mut);
    
  • Or you make your noncopyable and LockWrap types movable by switching to std::unique_lock instead of std::lock_guard:
    godbolt
    class noncopyable: base_token
    {
    protected:
      constexpr noncopyable() = default;
      ~noncopyable() = default;
      noncopyable( const noncopyable& ) = delete;
      noncopyable& operator=( const noncopyable& ) = delete;
      noncopyable( noncopyable&& ) = default;
      noncopyable& operator=( noncopyable&& ) = default;
    };
    
    class LockWrap : private noncopyable
    {
    public:
        explicit LockWrap(std::mutex & mut)
            : lock_(mut)
        {}
        std::unique_lock<std::mutex> lock_;
    };
    
    LockWrap genLockWrap(std::mutex & mut) {
      return LockWrap(mut);
    }
    
    // move-construction works now
    locks_held.emplace(1, genLockWrap(mut));
    locks_held.insert({1, genLockWrap(mut)});
    // etc...
    

Note that std::scoped_lock might be a better solution when you want to lock multiple mutexes, due to its built-in deadlock prevention.

std::mutex m1;
std::mutex m2;


std::scoped_lock lock(m1, m2);
Turtlefight
  • 9,420
  • 2
  • 23
  • 40