15

I want to return a std::vector. This std::vector may be accessed from other threads (read and write). How can I unlock my std::mutex just after the function has finished returning?

For example in something like:

// Value.cpp
std::vector<int> GetValue()
{
  std::lock_guard<std::mutex> lock(mutex);

  // Do super smart stuff here
  // ...

  return m_value;
}

// MyThread.cpp
auto vec = myVec.GetValue();

Now what if "Do super smart stuff here" is empty:

// Value.cpp
std::vector<int> GetValue()
{
  std::lock_guard<std::mutex> lock(mutex);
  return m_value;
}

// MyThread.cpp
auto vec = myVec.GetValue();

Then is the lock still mandatory? Why?

Korchkidu
  • 4,908
  • 8
  • 49
  • 69
  • 2
    The copy construction of `vec` happens outside the lock. If other threads could be modifying the referenced vector, you have a data race. – Casey Dec 09 '14 at 17:39
  • This will not work, you are returning some internal state (`m_value`) by reference. –  Dec 09 '14 at 17:40
  • @DieterLücking: indeed, I noticed this while adding the code actually^^' Even if m_value is a member (so it should work actually), returning by value fixes my problem (even though I still need to use the lock_guard). What I added is a unrealted issue actually. Thanks. – Korchkidu Dec 09 '14 at 17:41
  • 5
    If you return by value, all is fine. See http://stackoverflow.com/questions/8437763/c-return-value-created-before-or-after-auto-var-destruction –  Dec 09 '14 at 17:57
  • @DieterLücking: yes, indeed. I can't recall why I returned by const reference in the first place. Thanks! – Korchkidu Dec 09 '14 at 17:59

2 Answers2

23

Use a std::lock_guard to handle locking and unlocking the mutex via RAII, that is literally what it was made for.

int foo()
{
    std::lock_guard<std::mutex> lg(some_mutex);  // This now locked your mutex
    for (auto& element : some_vector)
    {
        // do vector stuff
    }
    return 5;
}  // lg falls out of scope, some_mutex gets unlocked

After foo returns, lg will fall out of scope, and unlock some_mutex when it does.

Cory Kramer
  • 114,268
  • 16
  • 167
  • 218
  • Then what if I have something like: auto vec = MyFunc(); operator= could be inserting returned vector from MyFunc() into vec while a thread modifies the returned vector? – Korchkidu Dec 09 '14 at 17:11
  • Then it sounds like you might have a design problem to think about. It's hard to give an answer without seeing a specific example. – Cory Kramer Dec 09 '14 at 17:22
10

This is the sort of question that print statements can really help with. For example:

#include <mutex>
#include <iostream>

std::mutex mut;

template <class Mutex>
class Lock
{
    Mutex& mut_;
public:
    ~Lock()
    {
        std::cout << "unlock\n";
        mut_.unlock();
    }

    Lock(const Lock&) = delete;
    Lock& operator=(const Lock&) = delete;

    Lock(Mutex& mut)
        : mut_(mut)
    {
        mut_.lock();
        std::cout << "lock\n";
    }
};

struct A
{
    ~A()
    {
        std::cout << "~A() : " << this << "\n";
    }

    A()
    {
        std::cout << "A() : " << this << "\n";
    }

    A(const A& a)
    {
        std::cout << "A(const A&) : " << this << ", " << &a << "\n";
    }

    A& operator=(const A& a)
    {
        std::cout << "A& operator=(const A&) : " << this << ", " << &a << "\n";
        return *this;
    }
};

A a;

A
get()
{
    Lock<std::mutex> lk(mut);
    return a;
}

int
main()
{
    std::cout << "Start\n";
    auto vec = get();
    std::cout << "End\n";
}

By making my own version of std::lock_guard, I can insert print statements to find out when the mutex gets locked and unlocked.

And by making a fake std::vector (called A above), I can insert print statements into the special members that I'm interested in. For me this outputs:

A() : 0x10fcfb188
Start
lock
A(const A&) : 0x7fff4ff06b28, 0x10fcfb188
unlock
End
~A() : 0x7fff4ff06b28
~A() : 0x10fcfb188

which clearly shows that the mutex is locked while the A at 0x10fcfb188 is being copied from.

The test can be altered for assignment with:

int
main()
{
    A vec;
    std::cout << "Start\n";
    vec = get();
    std::cout << "End\n";
}

which now outputs:

A() : 0x10d8a7190
A() : 0x7fff5235ab28
Start
lock
A(const A&) : 0x7fff5235ab18, 0x10d8a7190
unlock
A& operator=(const A&) : 0x7fff5235ab28, 0x7fff5235ab18
~A() : 0x7fff5235ab18
End
~A() : 0x7fff5235ab28
~A() : 0x10d8a7190

At first it looks like the assignment takes place outside the lock, and thus looks unsafe. However upon closer inspection one sees that the protected A at 0x10d8a7190 is copied to a temporary A inside the lock. The mutex is then unlocked and an assignment is made from the temporary to the local. No other thread could possibly reference the temporary. So as long as no other thread references vec, this is again safe.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577