3

I have a question about lock_guards and return values. I would like to illustrate my question with some code:

class Semaphore
{
public:
    Semaphore() = delete;
    Semaphore(int n);

    /**
     * Increases semaphore by one.
     */
    void up()
    {
        std::lock_guard<std::mutex> lg(m_);
        ++n_;
    }

    /**
     * Decreases semaphore by one.
     */
    void down()
    {
        std::lock_guard<std::mutex> lg(m_);
        --n_;
    }

    /**
     * Returns the underlying value of the semaphore.
     */
    int get1() const
    {
        std::lock_guard<std::mutex> lg(m_);
        int tmp = n_;
        return tmp;
    }

    /**
     * Returns the underlying value of the semaphore.
     */
    int get2() const
    {
        std::lock_guard<std::mutex> lg(m_);
        return n_;
    }

private:
    mutable std::mutex m_;
    int n_;
};

The above class is a simple implementation of a Semaphore. Which of the get-methods is thread-safe? Is get2 good enough or do I have to use get1? Do I have to copy the internal value n_ to a temporary variable or can I just return it right away?

This post boils-down to the question: Does the lock_guard protect my return value?

User12547645
  • 6,955
  • 3
  • 38
  • 69
  • I'm sure I already have found a duplicate of this question. But the answer is yes, the `std::lock_guard` will be destroyed after the return. – Fareanor Feb 21 '20 at 11:38
  • 1
    Probably a duplicate of: [Is a copy-on-return operation executed prior or after lock\_guard destructor?](https://stackoverflow.com/questions/53613641/is-a-copy-on-return-operation-executed-prior-or-after-lock-guard-destructor) except that `get1()` and `get2()` are replaced by `get_a()` and `get_b()` :p – Fareanor Feb 21 '20 at 11:40

2 Answers2

2

get2() is sufficient to protect the return value.

n_ is returned by value, therefore a copy of the variable is made to return to the caller with your return n_ statement. Your lock_guard will make sure that the n_ won't get changed until that copy is returned to the caller after which it gets destroyed thus releasing the lock.

Returning a copy is not incorrect, but will not give you any added benefits. Further in any case you need to maintain the lock until the copy is returned too. Therefore with get1 you pay for an additional copy assignment withouth much gain unless the compiler optimizes it out.

aep
  • 1,583
  • 10
  • 18
1

Both versions are equal. Return value is created before std::lock_guard is destroyed.

You can test this with next code snippet

#include <iostream>
#include <memory>
#include <mutex>

template <typename T>
struct GuardWrapper : std::lock_guard<T> {
    GuardWrapper(T &e) : std::lock_guard<T>(e) { std::cout << "Creating guard" << std::endl; }
    ~GuardWrapper() { std::cout << "Deleting guard" << std::endl; }
};

struct A {
    A() { std::cout << "Creating A" << std::endl; }
    A(const A& other) {
        *this = other;
        std::cout << "Copying A" << std::endl;
    }
    ~A() { std::cout << "Deleting A" << std:: endl; }
};

struct B {
    A test() {
        GuardWrapper<std::mutex> guard(m_);
        return a_;
    }
    A test_with_tmp() {
        GuardWrapper<std::mutex> guard(m_);
        A tmp = a_;
        return tmp;
    }

   private:
    std::mutex m_;
    A a_;
};

int main () {
    std::cout << "init" << std::endl;
    B b;
    std::cout << "try without temp" << std::endl;
    A a0 = b.test();
    std::cout << "try with temp" << std::endl;
    A a1 = b.test_with_tmp();
    std::cout << "cleanup" << std::endl;
}

output is

init
Creating A
try without temp
Creating guard
Copying A
Deleting guard
try with temp
Creating guard
Copying A
Deleting guard
cleanup
Deleting A
Deleting A
Deleting A
Tarek Dakhran
  • 2,021
  • 11
  • 21