Why does QSharedPointer<T>::create call destructor of incomplete object?

Question

I have following code example:

#include <QCoreApplication>
#include <QSharedPointer>
#include <QDebug>

#include <memory>

class A
{
public:
    A()
    {
        throw 1;
    }
    ~A() { qDebug() << "A destr"; }
};

int main(int argc, char* argv[])
{
    QCoreApplication a(argc, argv);

    try
    {
        //auto m1 = std::make_shared<A>();
        auto m2 = QSharedPointer<A>::create();
    }
    catch (...)
    {
        qDebug() << "catch!";
    }

    return a.exec();
}

The output for the above code is:

A destr
catch!

But if I uncomment the line with std::make_shared the output is following:

catch!

So why does QSharedPointer::create call destructor of incomplete object? Is that a bug or I'm missing something?

I tried it with MSVC2013 + Qt 5.5.1 and MSVC2015 + Qt 5.6 (built from sources). The result is the same.

Answer 1

It appears you have found a bug in Qt. I suggest you file a bug report and reference this somewhat related bug: https://bugreports.qt.io/browse/QTBUG-14637

The problem seems to be in http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h?h=v5.5.1#n420 - whose simplified code looks like this:

static inline QSharedPointer create()
{
    typedef QtSharedPointer::ExternalRefCountWithContiguousData<T> Private;
    typename Private::DestroyerFn destroy = &Private::deleter;

    QSharedPointer result(Qt::Uninitialized);
    result.d = Private::create(&result.value, destroy);

    new (result.data()) T();
    result.d->setQObjectShared(result.value, true);
    result.enableSharedFromThis(result.data());
    return result;
}

It's a little complicated with references to other functions (mostly in the same file), but it appears that deleter is stored in result before the constructor is called by placement new. When your constructor throws, your object is never completely constructed, but the QSharedPointer result is constructed already, and contains the deleter. From there it's a short hop to the deleter function:

static void deleter(ExternalRefCountData *self)
{
    ExternalRefCountWithContiguousData *that =
            static_cast<ExternalRefCountWithContiguousData *>(self);
    that->data.~T();
}

And now your destructor is called, despite your constructor never having completed. That's undefined behavior. If you're unlucky, this will corrupt your application state (because it goes against the rule that a destructor is only called if a constructor runs to completion--a rule some class types may rely on).

A possible fix (which I haven't tested, but you can) is:

static void noOpDeleter(ExternalRefCountData *self)
{
    Q_UNUSED(self);
}

static inline QSharedPointer create()
{
    typedef QtSharedPointer::ExternalRefCountWithContiguousData<T> Private;
    typename Private::DestroyerFn noDestroy = &noOpDeleter;
    typename Private::DestroyerFn destroy = &Private::deleter;

    QSharedPointer result(Qt::Uninitialized);
    result.d = Private::create(&result.value, noDestroy);

    new (result.data()) T();
    result.d->destroyer = destroy;
    result.d->setQObjectShared(result.value, true);
    result.enableSharedFromThis(result.data());
    return result;
}

If you can validate the above, you should feel free to weave it into a patch and submit it to the Qt bug tracker. Hopefully with a working patch attached they'll accept it promptly.