Why does QSharedPointer::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.

For reference the source code for your particular version of `QSharedPointer::create()` is here: http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h?h=v5.5.1#n420 - I assume you're building without `QT_SHAREDPOINTER_TRACK_POINTERS` defined? — John Zwinck, Dec 15 '15 at 14:34
And here is a possibly-related Qt bug filed five years ago (with no activity since): https://bugreports.qt.io/browse/QTBUG-14637 — John Zwinck, Dec 15 '15 at 14:43
@JohnZwinck Yes, it's not defined. As I understand this flag is used to debug multiple smart pointers owning the same object. Or should I give it a try anyway? — Yrchgrchh, Dec 15 '15 at 15:00
No need to define it--I was just trying to understand which code path you're actually using. It's clear now that there is a bug in Qt, which I have described in my answer below. — John Zwinck, Dec 15 '15 at 15:01

John Zwinck · Accepted Answer · 2015-12-15T14:58:30.303

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.

It doesn't take ill luck: calling the destructor of an object that isn't alive is undefined behavior. The lifetime of an object begins after its constructor completes successfully. In many cases, that UB may be innocuous, but it is UB regardless. — Yakk - Adam Nevraumont, Dec 15 '15 at 14:55
@Yakk: Thanks for that, I've updated my answer to reflect your point. — John Zwinck, Dec 15 '15 at 15:00

Youw · Answer 2 · 2017-04-17T13:42:48.473

-1

Finally, we will have it fixed! I gues it would be Qt 5.8.2 or Qt 5.9.

Thanks @JohnZwinck, your idea works just fine.

edited Apr 17 '17 at 13:42

answered Apr 11 '17 at 19:35

Youw

799
6
11

Why does QSharedPointer::create call destructor of incomplete object?

2 Answers2