QSharedData crash

Question

I have an infrequent but fairly consistent crash in my Qt 5.2.0 application which I am having a heck of a time diagnosing, but believe to be related to `QSharedData`. The application is highly multi-threaded, which is presumably part of the problem. The class in question is here: ``` class RouteData : public QSharedData { public: RouteData() : m_destAddress(0), m_valid(false), m_movingAverage(ROUTE_INITIAL_QUALITY) { } RouteData(const RouteData &other) : QSharedData(other), m_destAddress(other.m_destAddress), m_addresses(other.m_addresses), m_valid(other.m_valid), m_movingAverage(other.m_movingAverage), m_lastSuccess(other.m_lastSuccess), m_lastFailure(other.m_lastFailure) { } ~RouteData() { } quint16 m_destAddress; QList<quint16> m_addresses; bool m_valid; double m_movingAverage; QDateTime m_lastSuccess; QDateTime m_lastFailure; }; class Route { public: Route() { d = new RouteData; } Route(quint16 destAddress) { d = new RouteData; d->m_destAddress = destAddress; } Route(const Route &…

Accepted Answer

The instances of `QSharedData`, when accessed via a separate instance of `QSharedDataPointer`, can be, in fact, accessed from multiple threads at once, and it's safe. The shared data pointer will atomically take a copy of the referenced data, as needed.

So, as long as you're working on your own instance of the `Route` object, everything is fine. What you can't do is use a reference to an element held by a container. You can use a const-reference to a temporary object constructed from the one held by the container - that is, to a new instance.

Per the documentation:

Proper locking should be used when sharing an instance of an implicitly shared class between threads.

It is never correct to access a reference to a shared instance of an implicitly shared class without holding the lock.

You must always have a new instance. You can copy construct a temporary instance and then pass it via a const reference, for example as an argument in a function call. Such const-references-to-temporary-instances delay the destruction of the referenced temporary object until they are not needed anymore.

This applies to all implicitly shared classes in Qt 4 and Qt 5 -- whether coming from Qt proper (all containers!), or of your own design that leverages `QSharedDataPointer`.

So, this would be correct - you keep your own instance.

```
Route r(...);
QMutexLocker l(&routeMapMutex);
routeMap.insert(key, r);
l.unlock();
r.setDestination(...);
QMutexLocker l(&routeMapMutex);
routeMap.insert(key, r);
l.unlock();
```

As is this - again, you have your own instance.

```
QMutexLocker l(&routeMapMutex);
Route r = routeMap[key];
l.unlock();
if (r.isValid()) ...
```

But this is certainly incorrect, even though the reference is const.

```
QMutexLocker l(&routeMapMutex);
const Route & r = routeMap[key];
l.unlock();
if (r.isValid()) ...
```

But this is correct, since it is a const reference to a temporary instance, whose lifetime is prolonged as needed. So, you reference a new instance that nobody but you have access to.

```
QMutexLocker l(&routeMapMutex);
const Route & r = Route(routeMap[key]);
l.unlock();
if (r.isValid()) ...
```

This is also correct, since the call is guarded by the mutex:

```
void fun(const Route &);
QMutexLocker l(&routeMapMutex);
fun(routeMap[key]);
l.unlock();
```

My hunch is that somewhere in your code you access the reference to the value in the map (as opposed to a const-ref to a temporary) without holding the mutex.

It's also conceivable that you are having some other memory or threading bug that manifests itself in an unrelated place.

QSharedData crash

Solution

Problem