Thread safe singleton compile issues

[ingido]

Hello!
I'm trying to test thread safe singleton snippet.
But in the compilation i have an error:

error: invalid conversion from ‘Singleton<MyClass>::CreateInstanceFunction {aka MyClass* (*)()}’ to ‘QBasicAtomicPointer<void>::Type {aka void*}’ [-fpermissive]
   Singleton::create.store(create);
   ^~~~~~~~~

I can treat it as warning. But maybe there are a "clear" way to implement a thread safe singleton?
Thanks!

[mrjj]

It is a sign from above
    that you should much rather use
  some other method than singletons :)

http://softwareengineering.stackexchange.com/questions/40373/so-singletons-are-bad-then-what

[kshegunov]

The error's "fake", in the sense the compiler is actually whining for the implicit casting from Singleton<MyClass>::CreateInstanceFunction to void *. In any case as @mrjj said, do yourself a favor and don't use hacks. For one that mutex implementation (the one in qCallOnce) is totally unnecessary - it's just reinventing the wheel.

[ingido]

@mrjj, @kshegunov, thanks for the reply! I'm new to Qt and C++. Just trying to make one instance of NAM. I have a class - RequestHandler which inits (in constructor) a variable (called manager) with NAM instance inside class. Class is used fom easy request handling, building url, etc. Which way you can advice to make one instance NAM other way than singleton (with snippet or example)? Some examples of use below. Thanks in advance!

requesthandler.cpp

#include "requesthandler.h"

RequestHandler::RequestHandler(QObject *parent) :
    QObject(parent)
{
    this->manager = new QNetworkAccessManager(this);
}

QNetworkReply* RequestHandler::makeRequest(QString method)
{
    QNetworkRequest request;
    QUrl url = QUrl('localhost/api/' + method);
    request.setUrl(url);

    QNetworkReply *reply = this->manager->get(request);

    connect(reply, SIGNAL(readyRead()), this, SLOT(slotReadyRead()));
    connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(slotError(QNetworkReply::NetworkError)));
    connect(reply, SIGNAL(sslErrors(QList<QSslError>)), this, SLOT(slotSslErrors(QList<QSslError>)));

    return reply;
}

void RequestHandler::slotReadyRead()
{
    // ...
}

void RequestHandler::slotError(QNetworkReply::NetworkError error)
{
    // ...
}

void RequestHandler::slotSslErrors(QList<QSslError> errorList)
{
    // ...
}

QNetworkAccessManager* RequestHandler::getManager()
{
    return this->manager;
}

requesthandler.h

class RequestHandler : public QObject
{
    Q_OBJECT

public:
    explicit RequestHandler(QObject *parent = 0);
    QNetworkReply* makeRequest(QString method);
    QNetworkAccessManager* getManager();
private:
    QNetworkAccessManager* manager;

signals:

public slots:
    void slotError(QNetworkReply::NetworkError error);
    void slotSslErrors(QList<QSslError> errorList);
};

And somewhere in other classes:

#include "requesthandler.h"
void User::updateContactList()
{
    // Created a new instance
    RequestHandler *a = new RequestHandler;

    connect(a->getManager(), SIGNAL(finished(QNetworkReply*)), this, SLOT(contactListUpdated(QNetworkReply*)));

    a->makeRequest("user.contacts.get");
}

void User::contactListUpdated(QNetworkReply* reply)
{
    // processing...
    reply->deleteLater();
}

[kshegunov]

@ingido said in Thread safe singleton compile issues:

Just trying to make one instance of NAM.

Do so, you don't need to have a singleton for that. QCoreApplication is one such object (it's a pseudo-singleton), you create one instance but in principle it's not a real "singleton" as you can create multiple objects (although there's special care to warn you in that case, as well as if you didn't create an object).

Which way you can advice to make one instance NAM other way than singleton (with snippet or example)?

The so called "dependency injection pattern", which is just a fancy way of saying - pass a reference (or a pointer) to the needed object through the constructor of your class. Here's a very simple example of what it'd involve:

RequestHandler::RequestHandler(QNetworkAccessManager * nam, QObject *parent) :
    QObject(parent), manager(nam)
{
    Q_ASSERT(manager); //< Just for debug purposes, it gets removed when compiling in release mode. It will warn you if someone (you) pass a null pointer for the network manager.
}

And when you create your RequestHandler, you just pass the NAM instance as an argument:

int main(int argc, char ** argv)
{
    QApplication app(argc, argv);

    QNetworkAccessManager nam;
    RequestHandler handler(&nam); //< Passing the QNetworkAccessManager object to your handler.

    return QApplication::exec();
}

[matthew.kuiash]

@mrjj I'm of the philosophy that Qt really doesn't need singletons. More accurately it needs precisely 1 singleton and it already has one.

My preferred route is the derive from QApplication (QGuiApplication etc) and just put my objects in there (and implement the qApp macro)

Access to them is easy via a 'getter' or public variable.

Deriving from QApplication/GuiApp also lets one easily add serialisation etc.

Well, just my 2$. Seems to work for me.

[The Linthus]

Just for those how end here too with the same problem.

Casting would resolve the problem, it doesn't look right, but you will have no problem at all

Change

Singleton::create.store(create);

to

Singleton::create.store(((QBasicAtomicPointer<void>::Type)create));

[kshegunov]

@The-Linthus said in Thread safe singleton compile issues:

Casting would resolve the problem, it doesn't look right, but you will have no problem at all

Except the problem you use something that's completely unnecessary in 99.9999% of cases, introduces an application global state, more often than not leaks memory, needs special care to be made thread-safe, can't be made to have a specific order of initialization without putting a significant effort and so on ... but yes, except those things you will have no problem at all ...