QSharedPointer and incomplete (in this module ) types. Why failure when std:: pointers work?

AlexMal

First of all I faced the problem on Qt 4.8 -(did not try on Qt 5 which might have different behaviour.
see pseudo code:

main.cpp
{
...
   B b;
   QSharedPointer<Factory> factory =  QSharedPointer<Factory>(new Factory());
   b->setA (  factory ->createA() );
...
}

where

Factory::createA()
{
     QSharedPointer<A> a(    new A );
    return a;
}

assume B::setA does nothing or copy smartpointer.

In Factory.cpp and B:: cpp
there are #include "A.h"
but in main.cpp there is not .

You would expect that forward declaration from B and Factory headers) of A is sufficient,
But it is not. It does compile, but A will not be destroyed .
Welcome to memory leak :(

Using shared pointers from std will function as expected.

Question is why?
If QSharedPointer does not decrease reference counting to incomplete type it has to complain not introduce the memory leak;.

[Added code tags ~kshegunov]

VRonin

This post is deleted!

kshegunov

What's the prototype of B::setA? QSharedPointer works with forward declarations, so I'd guess you're using it incorrectly; consider giving a minimal example that can be compiled (and more importantly doesn't require us to guess about the types). Have a look also here, probably related to your case.

jdesjardins

Example:

a.h:

//#pragma once
#include <QDebug>

class A
{
public:
    virtual ~A()
    {
        qDebug() << "Destroy A";
    }
};

factorya.h:

#pragma once

#include <QSharedPointer>
#include <memory>

class A;

class FactoryA
{
public:
    QSharedPointer<A> createA();
    std::shared_ptr<A> createShared();
};

factorya.cpp

#include "factorya.h"
#include "a.h"

QSharedPointer<A> FactoryA::createA()
{
    return QSharedPointer<A>(new A());
}

std::shared_ptr<A> FactoryA::createShared()
{
    return std::shared_ptr<A>(new A());
}

main.cpp

#include "factorya.h"

//#include "a.h"

#include <memory>

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

    FactoryA factoryA;

    QSharedPointer<A> a = factoryA.createA();
    //std::shared_ptr<A> a = factoryA.createShared();
}

With std::shared_ptr, no leaks... with QSharedPointer, Destroy A is not printed.
Also, if we include "a.h" in main, there is no leak.

Using gcc4.8 and Qt 4.8.6

AlexMal

@kshegunov Thanks to jdesjardins who added a working code sample which demonstrates the problem.

Basically what happens as far as I can understand - when factory returns a shared pointer temporary copy of it is created in main and this copy will not be destroyed properly if main does not have #include.

,

Chris Kawa

This was fixed in Qt 5 and the above example works ok (prints the message).
The same issue (AFAIK also in Qt 5) affects QScopedPointer and is documented: Forward declared pointers.
So basically you need to have a non-inline constructor/destructor and a copy assignment operator (or make the class non-copyable).

AlexMal

@Chris-Kawa

Thanks for comment,
"or make the class non-copyable"
I do not think this affect anything, cause in application our classes are derived from QObject and macro Q_DISABLE_COPY makes them non-copyable . In fact the only thing which supposed to copied is a pointer, not class itself.

" non-inline constructor/destructor"
in our application that was the first thing I tried. - all the destructor bodies were put in cpp.
It is great that it is fixed ion Qt5, but we have a legacy code and the real problem is that gcc does not issue even warning (or we could not find the way to activate it). According to my impression QScopedPointer in this situation will not compile which is acceptable.

kshegunov

@jdesjardins said in QSharedPointer and incomplete (in this module ) types. Why failure when std:: pointers work?:

Also, if we include "a.h" in main, there is no leak.

You are obligated to include a.h in main, otherwise QSharedPointer<A> can't be instantiated at this line:

QSharedPointer<A> a = ...;

If it doesn't give you a compile time error, it should.

AlexMal

You are obligated to include a.h in main, otherwise QSharedPointer<A> can't be instantiated at this line:

This is not true. What has to be instantiated is an instance of QSharedPointer which needs only a pointer to A and thus forward declaration is sufficient.

Otherwise neither standard stl shared pointers nor QSharedPointer in Qt 5 would not work in this case.

kshegunov

@AlexMal said in QSharedPointer and incomplete (in this module ) types. Why failure when std:: pointers work?:

This is not true. What has to be instantiated is an instance of QSharedPointer which needs only a pointer to A and thus forward declaration is sufficient.

In Qt4 the default deleter doesn't do anything and the pointer is deleted directly, thus you're deleting a forward-declared type, meaning no destructors will be called. In Qt 5 the default deleter is a regular object which will actually delete the held pointer properly, as it's initialized (and thus instantiated) on object construction, but I may be wrong ...

Try adding a custom (trivial) deleter to your shared pointer whenever you create the objects.

AlexMal

@kshegunov said in QSharedPointer and incomplete (in this module ) types. Why failure when std:: pointers work?:

In Qt4 the default deleter doesn't do anything and the pointer is deleted directly, thus you're deleting a forward-declared type, meaning no destructors will be called. In Qt 5 the default deleter is a regular object which will actually delete the held pointer properly, as it's initialized (and thus instantiated) on object construction, but I may be wrong ...

You are probably right, but this is a problem of implementation, not a C++ standard or code.
At this moment I am trying to figure out what makes compiler stop issue a warning.
Depending on order of includes gcc might or may not isse warning: possible problem detected in invocation of delete operator

It seems it does if -isystem ...folder flag (which cmake puts before qt folder) is replaced with -I ...folder .
Hopefully I will find the way to control it.

kshegunov

@AlexMal said in QSharedPointer and incomplete (in this module ) types. Why failure when std:: pointers work?:

You are probably right, but this is a problem of implementation, not a C++ standard or code.

Only partly. This is a problem that's characteristic of templates. In the case you're describing you have two classes with the same name in two different modules, and there's no clash simply because none of them is exported. You have QSharedPointer<A> with known A within your module and then you have another QSharedPointer<A> with forward declared type within your program. Besides what I already suggested, you can also (explicitly instantiate and) export the template specialization from the library, if you have a compiler that supports it, this way you won't cause the compiler to instantiate the template (anew) in the application binary and thus you won't have the described problem.