CODEWITHSUNDEEP
c++c++11deadlock
PaperBirdMaster
PaperBirdMaster

Reputation: 13320

Shouldn't this code lead to a deadlock?

I have a class which contains a mutex and an object, each time I need to access the contained object, a method is called to lock the mutex and return te contained object, let's see the code:

template <typename MUTEX, typename RESOURCE>
class LockedResource
{
    using mutex_t    = MUTEX;
    using resource_t = RESOURCE;

    mutex_t    m_mutex;
    resource_t m_resource;

public:
        template <typename ... ARGS>
        LockedResource(ARGS &&... args) :
                m_resource(std::forward<ARGS>(args) ...)
        {}

    class Handler
    {
        std::unique_lock<mutex_t> m_lock;      // unique lock
        resource_t                &m_resource; // Ref to resource

        friend class LockedResource;

        Handler(mutex_t &a_mutex, resource_t &a_resource) :
            m_lock(a_mutex),       // mutex automatically locked
            m_resource(a_resource)
        { std::cout << "Resource locked\n"; }

    public:
        Handler(Handler &&a_handler) :
            m_lock(std::move(a_handler.m_lock)),
            m_resource(a_handler.m_resource)
        { std::cout << "Moved\n"; }

        ~Handler() // mutex automatically unlocked
        { std::cout << "Resource unlocked\n"; }

        RESOURCE *operator->()
        { return &m_resource; }
    };

    Handler get()
    { return {m_mutex, m_resource}; }
};

template <typename T> using Resource = LockedResource<std::mutex, T>;

The idea behind this code is to wrap an object and protect it from multiple access from multiple threads; the wrapped object have private visibility and the only way to access it is through the internal class Handler, the expected usage is the following:

LockedResource<std::mutex, Foo> locked_foo;
void f()
{
    auto handler = locked_foo.get(); // this will lock the locked_foo.m_mutex;
    handler->some_foo_method();
    // going out of the scope will call the handler dtor and
    // unlock the locked_foo.m_mutex;
}

So, if I'm not mistaken, calling the LockedResource::get method creates a LockedResource::Handle value which locks the LockedResource::m_mutex for the entire lifetime of the Handle... but I must be mistaken because the code below doesn't cause a deadlock:

LockedResource<std::mutex, std::vector<int>> locked_vector{10, 10};

int main()
{
/*1*/  auto vec = locked_vector.get(); // vec = Resource<vector>::Handler
/*2*/  std::cout << locked_vector.get()->size() << '\n';
/*3*/  std::cout << vec->size() << '\n';
    return 0;
}

I was expecting the line /*1*/ to lock the locked_vector.m_mutex and then the line /*2*/ try to lock the same already locked mutex causing deadlock, but the output is the following:

Resource locked
Resource locked
10
Resource unlocked
10
Resource unlocked

Here is the example code.

Upvotes: 4

Views: 5041

Answers (2)

ixSci
ixSci

Reputation: 13698

Well, quick tests show the following:

  1. GCC - shows the output which is shown in the question
  2. Clang - process killed on the online compiler I've used. So deadlock.
  3. MSVC2013 - "device or resource busy: device or resource busy" - is thrown. It detected an attempt to lock the already locked mutex on the same thread.

What standard has to say about it?

30.4.1.2.1/4 [ Note: A program may deadlock if the thread that owns a mutex object calls lock() on that object. If the implementation can detect the deadlock, a resource_deadlock_would_occur error condition may be observed. — end note ]

But according to 30.4.1.2/13 it should throw one of these:

— resource_deadlock_would_occur — if the implementation detects that a deadlock would occur. 
— device_or_resource_busy — if the mutex is already locked and blocking is not possible.

So the answer is yes, what you observe is an incorrect behavior. It should either block or throw but not proceed as nothing has happened.

The behavior observed is possible since you have UB in the code. According to 17.6.4.11, violation of a Requires clause is UB and in 30.4.1.2/7 we have the following requirement:

Requires: If m is of type std::mutex, std::timed_mutex, or std::shared_timed_mutex, the calling thread does not own the mutex.

Thanks to @T.C. for pointing out about UB.

Upvotes: 7

Roddy
Roddy

Reputation: 68023

I'm not familiar with this specific mutex/resource implementation, but it's common for such synchronization primitives to contain a LOCK COUNT, and to allow the same thread to lock the same object multiples times.

When the mutex has been unlocked the same number of times as it was locked, then another thread is free to lock it.

Upvotes: 0

Related Questions