How is QString's implicit sharing implementation thread-safe?

Question

I was looking through Qt 4.8.0's implementation of QString's implicit sharing, specifically how it's done it in a thread-safe manner. It seems that the key idea is to hold a reference count in an integer d->ref of type QBasicAtomicInt that can be incremented and decremented atomically. For referece:

inline QString::QString(const QString &other) : d(other.d)
{
  Q_ASSERT(&other != this);
  d->ref.ref();
}

inline QString::~QString()
{
  if (!d->ref.deref())
    free(d);
}

inline bool QBasicAtomicInt::deref()
{
    unsigned char ret;
    asm volatile("lock\n"
                 "decl %0\n"
                 "setne %1"
                 : "=m" (_q_value), "=qm" (ret)
                 : "m" (_q_value)
                 : "memory");
    return ret != 0;
}

Suppose the current value of d->ref in a QString object A is 1 and A.deref() is called. Isn't it possible that once the value of the ret != 0 expression (i.e. false) has been decided a different thread of execution could copy the QString, therefore incrementing its internal reference to 1? For example, this could happen if the second thread had a pointer to A and then did something like QString otherString = *Aptr;, which would call the copy constructor. In that scenario, the thread that saw deref() return false would free the shared memory, but the second thread would think it's still valid.

What am I missing? Is it that once you go multi-threaded taking pointers to these types of objects is inherently error-prone and should be avoided? Is the class thread safe as long as you only use its interface and refrain from taking pointers to them?

Since `deref` is called by the destructor, that situation would be UB from a dangling pointer. But I don't know if `deref` would be called from a different context. — Weak to Enuma Elish, Feb 14 '16 at 20:29

Jeremy Friesner · Answer 1 · 2016-02-15T00:07:07.080

Isn't it possible that once the value of the ret != 0 expression (i.e. false) has been decided a different thread of execution could copy the QString, therefore incrementing its internal reference to 1?

No, because if d->ref.deref() returns false, then we are guaranteed that there are no other pointers to d and therefore there is no way for any other thread to call ref() (or anything else) on that object.

Or, to put it another way, if there was a another QString object out there somewhere that was holding a pointer to the same shared-data object (d), then deref() would not have returned false in the first place, because (d)'s reference count would still be greater than zero.

What am I missing? Is the class thread safe as long as you only use its interface and refrain from taking pointers to them?

The QString class is thread-safe as long as each QString object is only accessed by a single thread at a time -- that is, your code can handle the QString objects just as if they weren't doing any shared-data tricks; the shared-data trick is transparent to the calling code (outside of the fact that your program uses less memory than it otherwise would have, of course :) )

Is it that once you go multi-threaded taking pointers to these types of objects is inherently error-prone and should be avoided?

Having two threads both trying to access the same QString object (A) at the same time would indeed be unsafe, just as it would be even if the QString class did not do any implicit data-sharing. So that's a no-no (unless you explicitly serialize the accesses using a QMutex or some other serialization mechanism). What you should do instead is give each thread its own separate QString object (and don't worry about increasing your memory usage because the implicit sharing trick avoids that).

There isn't another QString object, but there is a QString * that points to it and it gets de-referenced (QString callCopyConstructor = *Aptr) just in time to increment the QBasicAtomicInt. — MuchToLearn, Feb 15 '16 at 00:30
Indeed. Accessing a single QString object from two different threads without any synchronization is a classic race condition, and will lead to undefined behavior. See the final paragraph in my answer. — Jeremy Friesner, Feb 15 '16 at 04:39
@JeremyFriesner to be clear for all our readers: you are saying that even if what the threads have is *merely* constant read-only access to a QString, and none of them are modifying it, then that can still lead to crashes and therefore synchronization is required to prevent simultaneous reads. Am I right? — Patrick Parker, Sep 26 '19 at 16:22
@PatrickParker If the QString is never modified or deleted during the period when multiple threads can reference it, then it is effectively a read-only data structure and can be safely accessed without synchronization. In my post, I assumed that the QString would be (or could be) modified at some point while it was accessible to multiple threads; perhaps I should have made that assumption explicit. — Jeremy Friesner, Sep 26 '19 at 16:49

How is QString's implicit sharing implementation thread-safe?

1 Answers1