Reducing the scope of resource-manager objects with an if(true) clause?

Question

I have a resource storing object that lives in a multithread application. To (hopefully) ensure thread-safety I lock a mutex everytime I want to access a resource or insert a new one. For example, to insert a new resource:

void ResourceManager::insertResource(const std::string& id)
{
    // create the object with such ID
    Resource res = Resource(id);

    // ... more code on res

    // lock the mutex to insert the resource
    std::lock_guard<std::mutex> guard(mResourcesMutex);

    // insert the resource in a STL container
    mResources.insert(ResourceContainer::value_type(id, res));

    // ... more code that does not require the mutex lock
}

I want to lock_guard to be minimally scoped so its destructor is called as soon as possible and other threads can access the resources. In particular, I'd like to unlock the mutex after the mResources.insert(...) statement.

I thought about scoping the lock_guard using a naive if-statement:

    if(true)
    {
        std::lock_guard<std::mutex> guard(mResourceMutex);
        mResources.insert(ResourceContainer::value_type(id, res);
    }

but I don't know if its working. I find it hard to check if this is scoping the lock_guard right or if, on the other hand, the compiler just thinks I'm crazy and its optimizing the code taking out the if-statement.

My questions are:

• Is this working?
• Is it worth it? Will I notice a performance hit?
• Are there any better alternatives?

Answer 1

• Yes it works. As soon as you exit the scope, the objects local to that scope will be immediately destroyed.

• Yes it is worth it, especially in cases like your example (reducing the lifetime of an object to ensure a resource is released early -- but you can also use it just to reduce the scope of a temporary identifier and make it inaccessible after that, even though this use is a bit less interesting than your example).

• Yes there is a (slightly) better alternative: just use braces without the if clause:

.

{
    std::lock_guard<std::mutex> guard(mResourceMutex);
    mResources.insert(ResourceContainer::value_type(id, res);
}

A proper compiler will be able to optimize out the if (true) so the generated code will be identical, but the braces alone are much cleaner from a style point of view. Additionally the braces alone are a well known and immediately recognizable idiom, while your if (true) isn't and makes the reader wonder (even if briefly) "what is the purpose of that if? Oh wait, right, it's the usual scope-limiting idiom...".

Answer 2

Why you use if(true)? You can simply use block scope

{
    std::lock_guard<std::mutex> guard(mResourceMutex);
    mResources.insert(ResourceContainer::value_type(id, res);
}