Shouldn't this code be deadlocked?

I have a class that contains a mutex and an object, every time I need to access the contained object, a method is called to lock the mutex and return the object contained in the text to 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 has private visibility and the only way to access it is through the inner class Handler

, the expected usage is:

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, the method call LockedResource::get

creates a value LockedResource::Handle

that blocks LockedResource::m_mutex

for its entire lifetime Handle

... but I must be wrong because the code below does not cause deadlocks:

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 expected the row to /*1*/

lock locked_vector.m_mutex

and then the row /*2*/

would try to lock the same already locked mutex causing a deadlock, but the output is as follows:

Resource locked
Resource locked
10
Resource unlocked
10
Resource unlocked

      

  • Shouldn't the second ::get()

    get to a dead end?
  • Am I accessing the wrapped resource through the same lock, or am I misunderstanding something?

Here's an example .

+3


source to share


3 answers


Okay, quick tests show the following:

  • GCC - shows the result that is displayed in the question
  • Clang is a process killed in the online compiler I was using. So it's a dead end.
  • MSVC2013 - "Device or resource busy: device or resource busy" - selected. He detected an attempt to lock an already locked mutex on the same thread.

What standard should be said about this?

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

But according to 30.4.1.2/13 it should throw one of the following values:



— 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 are observing is wrong behavior. He must either block or throw, but not act as nothing happened.

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

Required : 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 @TC for pointing out UB.

+3


source


I am not familiar with this particular implementation of mutex / resource, but usually there is a LOCK COUNT for such synchronization primitives and allows the same thread to lock the same time intervals of objects.



When the mutex is unlocked the same number of times as it was locked, then another thread can lock it.

0


source


The documentation states:

If the lock is called by a thread that already owns the mutex, the behavior is undefined: the program may deadlock, or if the implementation can detect a deadlock, the resource_deadlock_would_occur error condition may occur.


By the way, because of things like this, I generally like to work with the mutex

shell subclass , which allows stuff to be asserted:

    /******************************************************************************
*******************************************************************************
******************************************************************************/
namespace detail
{


class mutex :
    public std::mutex
{
public:
#ifndef NDEBUG
    void lock()
    {
        using ms_t = std::chrono::milliseconds;

        assert(!locked_by_caller());
        std::mutex::lock();
        m_holder = std::this_thread::get_id(); 
    }
#endif // #ifndef NDEBUG

#ifndef NDEBUG
    void unlock()
    {
        assert(locked_by_caller());
        m_holder = std::thread::id();
        std::mutex::unlock();
    }
#endif // #ifndef NDEBUG

#ifndef NDEBUG
    bool locked_by_caller() const
    {
        return m_holder == std::this_thread::get_id();
    }
#endif // #ifndef NDEBUG

private:
#ifndef NDEBUG
    std::thread::id m_holder = std::thread::id();
#endif // #ifndef NDEBUG
};


/******************************************************************************
*******************************************************************************
******************************************************************************/
} // namespace detail

      

Note that it will catch your error here regardless of the underlying implementation mutex

.

0


source







All Articles