Segmentation fault when calling the constructor of a sibgelton class

FellowKrieger

Hello, I'm making a chat application in Qt with Qml and C++ classes and I'm having an issue when calling the member function of a singleton class. I'm getting a segmentation fault and the application crashes.

The application user is written as a singleton.
The .h:

class AppUser : public QObject
{
    Q_OBJECT

public:

    // singleton pattern
    static AppUser* getInstance();
    static QObject* qmlInstance(QQmlEngine *engine, QJSEngine *scriptEngine);
    AppUser(AppUser &A)= delete; // delete copy constructor
    void operator=(AppUser &A)=delete; // delete assign operation

    // getters
    static QString& getName();
    static QString& getPassword();
    static QString& getIPAddress();
    QTcpSocket& getTcpSocket();
    bool& getTcpConnectedState();


    // member functions inkobale through the QML side (through the VIEW)
    Q_INVOKABLE static void updateUserSettings(const QString &name,const QString &password,const QString &IPAddress);
    Q_INVOKABLE bool checkLogginInput(const QString &inputName,const QString &inputPassword,const QString &inputIP);


signals:
    // info notifications to user
    void connected();
    void disconnected();

public slots:
    // connection
    void connectToServerChat();
    void disconnectFromHost(); // close;

    // managing incoming data
//    void onReadyRead(); // IODevice


private:

    // singleton pattern
    static AppUser* m_app_user;
    explicit AppUser();
    ~AppUser();

    // setters
    static void setName(const QString &input_name);
    static void setPassword(const QString &input_password);
    static void setIPAddress(const QString &input_address);
    static void init_userApp_cient();
    void setConnectedInfo(const bool &input_connected_state);

    //attribute members
    QTcpSocket* m_socket;
    bool m_connected;
    static QString m_name; // = "username0";
    static QString m_password; //= "password0";
    static QString m_IPAddress; //= "1.0.0.0";

};

The crash occurs when we call the function updateUserSettings(const QString &name,const QString &password,const QString &IPAddress) from a qml by clicking on a buttton

Profilesettings.qml

onAccepted: loadAndRedirectNewUser()

                        function loadAndRedirectNewUser(){

                            AppUser.updateUserSettings(chosenName,chosenPassword,chosenIP);
                            rootStack.pop()
                        }

In he AppUser.cpp, the function is written:

void AppUser::updateUserSettings(const QString &name,const QString &password,const QString &IPAddress)
{

    AppUser::getInstance()->setName(name);
    AppUser::getInstance()->setPassword(password);
    AppUser::getInstance()->setIPAddress(IPAddress);


    qInfo("INFO : New user settings uploaded : name %s, password = %s, IP : %s", qPrintable(name),qPrintable(password),qPrintable(IPAddress));


With the constructor of the singleton

AppUser::AppUser()
{
qDebug() << "DEBUG : constructor of singleton AppUser";
m_socket = new QTcpSocket(this);
m_connected = false;
init_userApp_cient();

// connection
QObject::connect(m_socket, &QTcpSocket::connected, this, &AppUser::connectToServerChat);
QObject::connect(m_socket, &QTcpSocket::disconnected, this, &AppUser::disconnectFromHost);
//QObject::connect(m_socket, &QTcpSocket::readyRead, this, &AppUser::onReadyRead);

}

The related functions used by this function are, in AppUser.cpp

AppUser* AppUser::getInstance()
{
    if (m_app_user == nullptr) // avoid creation of new instances
        m_app_user = new AppUser;

    return m_app_user; // instance
}

void AppUser::setName(const QString &input_name)
{
    m_name = input_name;
}

void AppUser::setPassword(const QString &input_password)
{
    m_password = input_password;
}

void AppUser::setIPAddress(const QString &input_address)
{
    m_IPAddress = input_address;
}

in the main.cpp we declare the singleton to be used as a qml singleton

#include "appuser.h"
...

qmlRegisterSingletonType<AppUser>("AppUser",1,0,"AppUser", &AppUser::qmlInstance);

qml Instance is writtten in Appuser.cpp:

QObject* AppUser::qmlInstance(QQmlEngine *engine, QJSEngine *scriptEngine)
{
    Q_UNUSED(engine);
    Q_UNUSED(scriptEngine);
    // C++ and QML instance they are the same instance
    return AppUser::getInstance();
}

When I run my program,
the user fills a form in the qml and validate on clicking this button. Then the application crashes.

When I ran the debugger with a breakpoint on calling the member function update AppUser.updateUserSettings(chosenName,chosenPassword,chosenIP);
.I saw tha the program entering the constructor AppUser::AppUser(), and I got the message:

And I got my logs from the Qdebug I put in the constructor called like hundreds of times in a row.

DEBUG : constructor of singleton AppUser.

Then the application crashes.

I guess I am doing something wrong with the pointers I use for the singleton, some missuse of static variables or the proper use of the singleton with the qml engine.

Excuse-me if my question is kind of lengthy, I hope I kept everything here compact enough.

jsulm

@FellowKrieger If your application is crashing then please first run it through debugger and check the stack trace to see where exactly it is crashing. You can also post the stack trace here, so others can also take a look.

sierdzio

You should set Cpp ownership for your singleton, otherwise QML engine will assume it can delete the pointer: https://doc.qt.io/qt-6/qjsengine.html#setObjectOwnership

Since you are registering an existing instance, you can use qmlRegisterSingletonInstance, too.

The crash might happen because your m_app_user is not initialized to nullptr, so it will take a random value (thanks, c++ :( ). So your call to getInstance() erroneously assumes that the object exists when it does not exist.

You also don't initialize other member variables, this is dangerous. As is returning non-const references in getters.

Another issue is that you do not call QObject constructor when constructing your singleton object.

FellowKrieger

@sierdzio
Thanks the application does not crash any more.
I also see my qDebug log only once.

As you said I initialized the variables of the singleton, and I call the constructor of the QObject class.

// .h
explicit AppUser(QObject* parent=nullptr);

// .cpp

AppUser::AppUser(QObject* parent) : QObject (parent)
{

    qDebug() << "DEBUG : constructor of singleton AppUser";
    m_socket = new QTcpSocket(this);
    m_connected = false;
    m_app_user = nullptr;
    m_name=QStringLiteral("username0") ;
    m_IPAddress= QStringLiteral("password0");
    m_password = QStringLiteral("1.0.0.0");

    // connection
    QObject::connect(m_socket, &QTcpSocket::connected, this, &AppUser::connectToServerChat);
    QObject::connect(m_socket, &QTcpSocket::disconnected, this, &AppUser::disconnectFromHost);
    //QObject::connect(m_socket, &QTcpSocket::readyRead, this, &AppUser::onReadyRead);

}

// the variables are also declared at the top of the .cpp

AppUser* AppUser::m_app_user = nullptr;
QString AppUser::m_name=QStringLiteral("username0") ;
QString AppUser::m_IPAddress= QStringLiteral("password0");
QString AppUser::m_password = QStringLiteral("1.0.0.0");

I set the Ownership of the singleton in the getInstance method, is it correct?

AppUser* AppUser::getInstance()
{
    if (m_app_user == nullptr) // avoid creation of new instances
        m_app_user = new AppUser;

    QQmlEngine::setObjectOwnership(m_app_user, QQmlEngine::CppOwnership);
    return m_app_user; // instance
}

sierdzio

@FellowKrieger yes, correct. You can do it under the if, it needs to be done only once.

SimonSchroeder

In modern C++ there is a better way to implement the singleton pattern. The class variable (in theory) can have problems when using multiple threads. A better approach is to ditch the class variable and do it like this:

AppUser* AppUser::getInstance()
{
  static AppUser app_user = AppUser();
  return &app_user;
}

Modern C++ makes sure that static variables inside functions will be initialized only once (even with multiple threads). It also makes sure that you can only access the singleton instance through this function.

You might also want to think about moving setObjectOwnership inside the AppUser constructor.