Mutex in operator []

Question

I want to do something like that:

class BlockingVector{
protected:
    std::vector<int> m_vector;
    std::mutex m_mutex;
public:
    int & operator [](size_t p_index){
        std::lock_guard<std::mutex> lock(m_mutex);
        return m_vector[p_index];
    }
};

I know, this is totally wrong. Is there a way to overloading operator [] with mutex?

As Nikos C says: It returns a reference to the element, not a copy. Thus it's not thread-safe to use that element.

Why would it be wrong? What is the problems you have with the code? How do you use it? Please [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask), and learn how to create a [Minimal, **Complete**, and Verifiable Example](http://stackoverflow.com/help/mcve). — Some programmer dude, Nov 27 '17 at 04:26
@Yuushi It returns a reference to the element, not a copy. Thus it's not thread-safe to use that element. — Nikos C., Nov 27 '17 at 04:26
When 2 threads access 1 element? Or BlockingVector bv; vb[1]= bv[1]+1; I do not know what will happen — Hoàn Trần, Nov 27 '17 at 04:28
Unless you want to disallow setting a specific value using `operator[]` (which is simple, just return by value) there's no way to accomplish it without a wrapper object. You could make a wrapper that wraps the reference, and also contains a reference to the mutex. Then in its `operator=` function you lock the mutex before assigning. — Some programmer dude, Nov 27 '17 at 04:37
Also, I would recommend you make an overload of `operator[]` that *do* return by value, and is marked as `const`: `int operator[](const size_t p_index) const { ... }`. Then the compiler can choose which one fit the best depending on how it is used. With the example `bv[1] = bv[1] + 1` the constant and by-value-returning function would probably be used for the second access. — Some programmer dude, Nov 27 '17 at 04:40
so can not get the thread-safe with the operator[] as above? — Hoàn Trần, Nov 27 '17 at 04:41
@HoànTrần Without knowing your constraints, requirements and what could be allowed: No. — Some programmer dude, Nov 27 '17 at 04:42

score 1 · Accepted Answer · answered Nov 27 '17 at 05:33

You could define a 'locked_reference' helper class that acquires the lock in its constructor and releases in the destructor and use that as the return value of operator[]:

template<class T> class BlockingVector{
private:
    std::vector<T> m_vector;
    std::recursive_mutex m_mutex;
    class locked_ref {
        T &ref
        lock_guard<std::recursive_mutex> lock;
    public:
        locked_ref(T &r, std::recursive_mutex &m) : ref(r), lock(m) {}
        locked_ref(locked_ref &&) = default;
        const T &operator=(const T &v) const { return ref = v; }
        operator T() const { return ref; }
    }; 
public:
    locked_ref operator [](size_t p_index){
        return locked_ref(m_vector[p_index], m_mutex); }
};

You need a recursive_mutex as a thread will need to lock it multiple times to evalute something like:

v[i] = v[j] + v[k];

and there's a danger of deadlock if two threads operate on two vectors simultaneously.

Nikos C. · Answer 2 · 2017-11-27T04:57:30.000

No. The caller needs to acquire a lock since it gets a reference to the data instead of a copy. You can't somehow make reference to an int thread-safe for all code that uses it.

If you need a thread-safe operation on the data and don't want the caller to be responsible, you have two options: a) add that operation in the API of your class, or b) implement a wrapper type around the int reference.

A wrapper can be complicated to implement correctly, as you now need to worry about every possible corner case. Is it copyable? Movable? Both in a thread-safe manner?

So I'd opt for adding the operation as an API. Like:

class BlockingVector{
    // ...

    void setVal(size_t index, int newVal)
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        m_vector[index] = newVal;
    }

    // Change: return copy, not reference.
    int operator [](size_t p_index) const {
        std::lock_guard<std::mutex> lock(m_mutex);
        return m_vector[p_index];
    }
};

However, this can be a confusing API. So I'd suggest not overloading operator[] instead use a normal function:

int getVal(size_t p_index) const {
    std::lock_guard<std::mutex> lock(m_mutex);
    return m_vector[p_index];
}

Note that to be able to lock the mutex in a const function, you need to make it mutable:

class BlockingVector{
    // ...
    mutable std::mutex m_mutex;

You should read this for more information on mutexes and const functions:

Should mutexes be mutable?

A little wonder. Function may be const when using mutex( std::lock_guard lock(m_mutex);) ? — Hoàn Trần, Nov 27 '17 at 04:51
@HoànTrần It's common to mark mutexes as `mutable`. In this case, you'd need to declare the mutex with `mutable std::mutex m_mutex;`. Then you can lock it even in `const` functions. However, I recommend you read this: https://stackoverflow.com/questions/4127333/should-mutexes-be-mutable#4128689 — Nikos C., Nov 27 '17 at 04:55

Mutex in operator []

2 Answers2