Using QString causes crash after QMap::remove

Question

I have the following code:

class NamedObjectContainer {
    //...
    QMap<QString, SomeStruct> mUsed;
    //...
};

const StoredObject* NamedObjectContainer::use(const QString& name, const QString& userId)
{
    qDebug()<<userId;
    mUsed.remove(userId);
    qDebug()<<userId;
    //...
}

Here I'm trying to remove element from QMap by key (userId). Element is removed correctly. But surprisingly it crashes printing userId after QMap::remove.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb5b2c6c0 (LWP 24041)]
0xb5fe899c in memcpy () from /lib/i686/nosegneg/libc.so.6
(gdb) where
#0  0xb5fe899c in memcpy () from /lib/i686/nosegneg/libc.so.6
#1  0xb7263246 in QString::append () from /home/osmin/stand_cl_dir/Qt4_x86-linux/lib   /libQtCore.so.4
#2  0xb72b6641 in ?? () from /home/osmin/stand_cl_dir/Qt4_x86-linux/lib/libQtCore.so.4
#3  0xb72b218b in QTextStream::operator<< () from /home/osmin/stand_cl_dir/Qt4_x86-linux/lib/libQtCore.so.4
#4  0xb6524740 in QDebug::operator<< () from /usr/lib/libqxmlrpc.so.1
#5  0xb62b5cc0 in tabexchange::NamedObjectContainer::use (this=0x9e2fb08, name=@0xbffe85e4, userId=@0xa12b780) at namedcontainer.cpp:208

What can cause the problem? I'm using Qt 4.4.3

Answer 1

To elaborate on @TI's comment...

QString is an implicitly shared type. Each new copy made of a QString object increases the reference count under the hood, and when the count goes to zero it is destroyed.

What probably happened here is that there was an initialization routine which made a QString instance, passed it in as the key, and the map made a copy. (This does not copy the data, merely increases the shared count.) Then the initialization routine destroyed its instance, so the only shared instance left is the one stored in the map with a share count of 1.

Later you probably used something like the QMap::iterator::key() to get a const reference to the string key in the map, passed in as userId. That wouldn't create any new instance of a QString to add to the shared count, but rather points to the one the map owns. So when the map lets go of it...it's destroyed and now userId is a dangling reference.

(Note: You don't say what's in SomeStruct. But if through it an instance of a matching string to the key could be reached which would be destroyed when the map value's SomeStruct is destroyed, then passing in a reference to such a string as userId could cause an analogous problem.)

One thing implicit sharing throws into the mix is that sometimes it hides bugs of this nature--which would be made more obvious without implicit sharing. Yet it makes the solution "inexpensive": when you extract the key to pass in, copy it into a local variable instance...and pass a const reference to that variable to this routine. That won't actually copy the data, but it will make userId safe because there will be one more shared count keeping it alive.

This helps implement the more generally good protocol: passing a reference type to a routine should mean that you can guarantee the lifetime of the referenced object for the entire runtime of the function you are calling. If that's ever in doubt, make a copy and pass a reference to the copy instead.

(Note: In the future, try using the Simple, Self Contained, Correct Example format with both the add and remove included, it can lead to finding smoking guns on your own more easily. And without it, we can only make educated guesses on the problem...it could be caused by something else in your program entirely!)