CODEWITHSUNDEEP
c++multithreadingc++11
Ghassen Hamrouni
Ghassen Hamrouni

Reputation: 3168

Safe destruction of a map containing shared_ptr

I was reading the Mir project source code and I stumbled upon this piece of code :

void mir::frontend::ResourceCache::free_resource(google::protobuf::Message* key)
{
    std::shared_ptr<void> value;
    {
        std::lock_guard<std::mutex> lock(guard);

        auto const& p = resources.find(key);

        if (p != resources.end())
        {
            value = p->second;
        }

        resources.erase(key);
    }
}

I have seen this before in other projects as well. It holds a reference to the value in the map before its erasure, even when the bloc is protected by a lock_guard. I'm not sure why they hold a reference to the value by using std::shared_ptr value.

What are the repercussions if we remove the value = p->second ?

Will someone please enlighten me ?

This is the code http://bazaar.launchpad.net/~mir-team/mir/trunk/view/head:/src/frontend/resource_cache.cpp

Upvotes: 1

Views: 389

Answers (2)

Dave S
Dave S

Reputation: 21058

The goal is to move the actual execution of the shared_ptr deleter till after the lock is released. This way, if the deleter (or the destructor using the default deleter) takes a long time, the lock is not held for that operation.

If you were to remove the value = p->second, the value would be destroyed while the lock is held. Since the lock protects the map, but not the actual values, it would hold the lock longer than is strictly necessary.

Upvotes: 2

rodrigo
rodrigo

Reputation: 98348

My guess it that this is done to avoid running the destructor of value inside the locked code. This lock is meant to protect the modification of the map, and running some arbitrary code, such as the destructor of another object with it locked, is not needed nor wanted.

Just imagine that the destructor of value accesses indirectly, for whatever reason to the map, or to another thread-shared structure. There are chances that you end up in a deadlock.

The bottom end is: run as little code as possible from locked code, but not less. And never call a external, unknown, function (such as the shared_ptr deleter or a callback) from locked code.

Upvotes: 3

Related Questions