Using 'mutable' for asynchronously-populated cache in a const method

Question

I'm concerned that I'm breaking the contract of mutable which I use for caching information in a data model that performs on-demand requests asynchronously. The data model happens to be Qt, although that's not a particularly important fact.

class MyDataModel : public QAbstractItemModel
{
public:
    QVariant data( const QModelIndex & index, int role ) const override;

private:
    void SignalRowDataUpdated( int row ) const;
    mutable SimpleRowCache mCache;
};

When data() is called, I check the cache to see if we have it. If not, I return empty data immediately (to avoid blocking the UI) and also send an asynchronous request to the API to populate the cache. Since data() must be const, this requires that mCache is mutable. The guts of data() looks like this:

RowData row_data = mCache.Get( row );
if( !row_data )
{
    // Store empty data in cache, to avoid repeated API requests
    mCache.Set( row, RowData() );

    // Invoke API with a lambda to deliver async result.  Note: 'this' is const
    auto data_callback = [this, row]( RowData data )
    {
        mCache.Set( row, std::move(data) );
        SignalRowDataUpdated( row );
    };
    DataApi::GetRowData( row, data_callback );

    return QVariant::Invalid;
}
return row_data[ column ];

My concern is that the data model object's logical constness is being violated here: Calling data() for some index can directly result in a future call with the same parameters returning a different value.

Is this A Bad Idea? And is there a common pattern / paradigm for doing it "correctly"?

---

Footnote: I have a similar issue with SignalRowDataUpdated(). This is actually a wrapper around emitting a Qt signal: emit dataChanged( from, to ), which is a non-const call. I've handled that one by capturing this in a lambda at construction time allowing me to call the non-const method from a const function. I don't feel proud of this =(

Answer 1

First of all, const member function today is not about logical constness. It is about thread safety (see https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/).

The main thing is that if you make member function const, you should guarantee that if it's invoked from several threads, there won't be race conditions.

If you don't like mutable cache inside your class-definition, use indirection, making a smart pointer to your cache inside your class definition instead. You won't need mutable and compiler won't be upset about your function being const :)

Answer 2

From section [class.this] in the C++ language standard,

If the member function is declared const, the type of this is const X*

So the const changes the type of this; there is nothing (that I've found) about "logical constness" of a class. Since this is a const pointer to MyDataModel, what can you do with it? [decl.type.cv] tells us

Except that any class member declared mutable can be modified, any attempt to modify a const object during its lifetime (3.8) results in undefined behavior.

Making changes to mCache within data() is allowed since it is declared mutable.

There a not a requirement that a const member function must return the same value when called with the same parameters, as the internal state could get changed by updating mutable members or intervening calls to non-const member functions.

In summary, as long as your calling code can handle the three different returned values (QVariant::Invalid, an empty RowData object if the data is currently being loaded, and a loaded row) you shouldn't have any issues with the member function declaration.

Where you can have an issue is in your callback, when you move the newly loaded data into your cache. If another thread tries to load the data for that row at the same time, it can get a RowData object that a mix of an empty row and a loaded one.

In SignalRowDataUpdated, why are you doing the indirection thru a lambda, instead of doing a const_cast on this?