When lock does not lock — C++ story

Author:	Wojciech Muła
Added on:	2018-03-26

A few days ago I had compiled fresh GCC 8.0 from trunk and then compiled our product, just to see what we will have to fix in the future. And I found a nasty mistake.

Lets start with a class that perhaps every project has.

class Resource {
    int resource;
    std::shared_mutex mutex;
public:
    int get() {
        std::shared_lock<std::shared_mutex>(mutex);
        return resource;
    }
};

At first glance get() protects the shared resource; thanks to RTTI we don't have to care about locking and unlocking our mutex, as std::shared_lock internals care about this.

However, the code doesn't work that way. Indeed, the line

std::shared_lock<std::shared_mutex>(mutex);

declares std::shared_lock variable. But the name of variable is mutex and the lock is constructed with no associated mutex. Thus, the lock doesn't lock anything.

Of course, the correct declaration should be:

std::shared_lock<std::shared_mutex> lock(mutex);

Can we detected this kind of mistake? Yes, GCC has flag -Wshadow that warns when "a local variable or type declaration shadows another variable, parameter, type, or class member (in C++), or whenever a built-in function is shadowed":

file.cpp: warning: declaration of ‘mutex’ shadows a member of ‘Resource’ [-Wshadow]
     std::shared_lock<std::shared_mutex>(mutex);

However, I found the mistake thanks to more aggressive -Wparentheses flag in GCC 8:

file.cpp: warning: unnecessary parentheses in declaration of ‘mutex’ [-Wparentheses]
     std::shared_lock<std::shared_mutex>(mutex);