Message Handler crashes if installed on sepparate class?



  • Whenever there is a message sent to my custom message handler, it is crashing the application at the marked line through a segmentation fault error.

    main.cpp
    
    void myMessageOutput(QtMsgType type, const QMessageLogContext &context, const QString &msg)
    {
        QStringList data;
        data.append(context.category);
        data.append(context.file);
        data.append(context.function);
        data.append(QString(context.line));
        data.append(QString(context.version));
        Logger::get_obj()->myMessageOutput(type, data, msg);
    }
    
    int main(int argc, char *argv[])
    {
        QApplication a(argc, argv);
        a.setApplicationName("MyApp");
        a.setWindowIcon(QIcon(":/Icons/MyApp.png"));
    
        Logger log;
        qInstallMessageHandler(myMessageOutput);
        log.turnOnDebug();
    
        MySplashScreen splash;
        splash.show();
        a.processEvents();
    
    ...
    
        splash.close();
        MainWindow *w = new MainWindow();
    
        int errorCode = a.exec();
        delete w;
        session.destruct();
        qDebug() << "\n*** Application exited with error code " << errorCode << " ***";
        return errorCode;
    }
    
    logger.h
    
    #ifndef LOGGER_H
    #define LOGGER_H
    
    #include <QObject>
    #include <QFile>
    #include <QTextStream>
    #include <QMutex>
    
    class Logger : public QObject
    {
        Q_OBJECT
    public:
        void myMessageOutput(QtMsgType type, QStringList context, const QString msg);
        explicit Logger(QObject *parent = 0);
        ~Logger();
        static Logger *get_obj() {return ptr;}
        static void turnOnDebug() {get_obj()->debugMode = true;}
        static void turnOffDebug() {get_obj()->debugMode = false;}
        static void moveLogToSession(QString sessionName);
    
    private:
        QFile *logFile;
        QTextStream *stream;
        QMutex *mutex;
        bool debugMode;
        bool errorMsg;
        static Logger *ptr;
    
        void write(QString str);
    
    signals:
        void SaveSession();
        void FatalSaveSession();
    
    public slots:
    
    };
    
    #endif // LOGGER_H
    
    
    logger.cpp
    
    #include <QDir>
    #include <QFile>
    #include <QMessageBox>
    #include <QApplication>
    #include "logger.h"
    #include "systemvariables.h"
    
    Logger *Logger::ptr = NULL;
    
    Logger::Logger(QObject *parent) : QObject(parent)
    {
        logFile = NULL;
        stream = NULL;
        mutex = NULL;
        debugMode = false;
        errorMsg = false;
    
        ptr = this;
        QString path = SystemVariables::getAppData();
        QDir dir = QDir(path);
        if(dir.exists())
        {
            path.append("/MyApp/Logs");
            dir = QDir(path);
            if(!dir.exists())
                dir.mkpath(path);
    
            logFile = new QFile(path + "/Session.log");
            if(logFile->exists())
                logFile->remove();
    
            if(!logFile->open(QFile::WriteOnly | QFile::Text))
            {
                qFatal("Could not create log file.");
            }
    
            stream = new QTextStream(logFile);
            stream->setRealNumberNotation(QTextStream::SmartNotation);
            stream->setRealNumberPrecision(15);
            *stream << "*** MyApp Session Begins ***\n";
        }
        else
            qFatal("Could not create log file.");
    
        mutex = new QMutex();
    }
    
    Logger::~Logger()
    {
        if(stream)
        {
            stream->flush();
            delete stream;
            stream = NULL;
        }
    
        if(logFile)
        {
            logFile->flush();
            logFile->close();
            delete logFile;
            logFile = NULL;
        }
    
        if(mutex)
        {
            delete mutex;
            mutex = NULL;
        }
    
        ptr = NULL;
    }
    
    void Logger::myMessageOutput(QtMsgType type, QStringList context, const QString msg)
    {
       mutex->lock();
       QString str = "";
        switch (type)
        {
            case QtDebugMsg:
            if(!debugMode)
            {
                mutex->unlock();
                return;
            }
            write(msg);
            break;
    
            case QtInfoMsg:
            str.append("\n*** Information ***\n");
            str.append(msg + "\n");
            str.append("Category: " + context.at(0) + "\n");
            str.append("File: " + context.at(1) + "\n");
            str.append("Function: " + context.at(2) + "\n");
            str.append("Line: " + context.at(3) + "\n");
            str.append("Version: " + context.at(4));
            str.append("\n*** Information Complete ***\n");
            write(str);
            break;
    
            case QtWarningMsg:
            if(!(context.at(2).contains("setGeometry")))
            {
                str.append("\n*** Warning ***\n");
    ...
                str.append("\n*** Warning Complete ***\n");
                write(str);
                errorMsg = true;
                emit Logger::ptr->SaveSession();
            }
            break;
    
            case QtCriticalMsg:
            str.append("\n*** Critical ***\n");
    ...
            str.append("\n*** Critical Complete ***\n");
            write(str);
            errorMsg = true;
            emit Logger::ptr->SaveSession();
            break;
    
            case QtFatalMsg:
            str.append("\n*** Fatal ***\n");
    ...
            str.append("\n*** Fatal Complete ***\n");
            write(str);
            errorMsg = false;
            emit Logger::ptr->FatalSaveSession();
            QApplication::exit(-2);
        }
        Logger::mutex->unlock();
    }
    
    void Logger::write(QString str)
    {
        if(!stream)
            return;
    
        if(str.isEmpty())
            return;
    
        (*stream) << str; //!!!!!!!!!!!!CRASHES HERE***********
        stream->flush();
    }
    
    void Logger::moveLogToSession(QString sessionName)
    {
        QMutex *myMutex = get_obj()->mutex;
        myMutex->lock();
    
        QTextStream *myStream = get_obj()->stream;
        myStream->flush();
        QFile *myLogFile = get_obj()->logFile;
        myLogFile->flush();
        myLogFile->close();
        delete myStream;
    
        QString path = SystemVariables::getAppData() + "/MyApp/Logs";
        if(!myLogFile->open(QFile::ReadOnly | QFile::Text))
        {
            QString errtitle = "FATAL ERROR!";
            QString errtxt = myLogFile->errorString() + "\nCould not open the log file: ";
            errtxt.append(path + "/Session.log");
            errtxt.append(".\nApplication Abort!");
            QMessageBox *mb = new QMessageBox(QMessageBox::Critical, errtitle, errtxt);
            mb->exec();
            delete mb;
            QApplication::exit(-1);
        }
        QTextStream in(myLogFile);
        QString data = in.readAll();
        myLogFile->close();
        myLogFile->remove();
        delete myLogFile;
    
        path = SystemVariables::getAppData() + "/MyApp/Sessions/" + sessionName;
        myLogFile = new QFile(path + "/Session.log");
        if(!myLogFile->open(QFile::WriteOnly | QFile::Text))
        {
            QString errtitle = "FATAL ERROR!";
            QString errtxt = myLogFile->errorString() + "\nCould not write to the log file: ";
            errtxt.append(path + "/Session.log");
            errtxt.append(".\nApplication Abort!");
            QMessageBox *mb = new QMessageBox(QMessageBox::Critical, errtitle, errtxt);
            mb->exec();
            delete mb;
            QApplication::exit(-1);
        }
        QTextStream out(myLogFile);
        out << data;
        out.flush();
        myLogFile->flush();
    
        myStream = new QTextStream(myLogFile);
        myStream->setRealNumberNotation(QTextStream::SmartNotation);
        myStream->setRealNumberPrecision(15);
    
        myMutex->unlock();
    }
    

    I would really be thankful if someone could help me by pointing out my mistakes.


  • Qt Champions 2017

    Hello,
    If I had to guess the problem is here:

    void Logger::moveLogToSession(QString sessionName)
    {
        ...
        QTextStream *myStream = get_obj()->stream;
        myStream->flush();
        ...
        delete myStream;  // < You delete the get_obj()->stream instance (and then look down)
        ...
        myStream = new QTextStream(myLogFile); // < You are not updating the pointer you're holding in get_obj()->stream
        myStream->setRealNumberNotation(QTextStream::SmartNotation);
        myStream->setRealNumberPrecision(15);
        ...
    }
    

    As additional advice, either hold your pointers in QScopedPointer, QSharedPointer, QPointer and the like (Qt provides a lot of pointer wrappers for different cases), or best just create the variables on the stack. If you just declare the variables in your class (and not pointers holding them) you don't need to clean them up manually. Additionally you can use QMutexLocker utility class to ensure that the mutex will be unlocked when you leave your current stack frame. In your current code if any of the methods throws an exception ... well you're in trouble.

    Kind regards.



  • @kshegunov Thank you very much!!!
    That resolved the crash!
    Thanks for the additional advice as well, I will follow that.


  • Qt Champions 2017

    @CAD_coding
    Hello,
    I'm glad it worked. I get that it's seductive to use new (and in many languages this is the only way) to get your objects, but in my opinion it's mostly overused, similarly to the singleton and some other patterns. I'm very happy that Qt allows for old-school-minded people like me to create their objects on the stack ... :)

    Kind regards.



  • @kshegunov
    Thank you for your useful advice.
    I use dynamic memory because it prevents me from Stack overflow. I don't know if that is unnecessary, but just being careful.
    It also allows me to replace objects easily, but as the above bug suggests, that may not always be helpful.


  • Qt Champions 2017

    @CAD_coding said:
    I'd be really glad if it's indeed useful. :)

    I use dynamic memory because it prevents me from Stack overflow.

    I don't understand what you mean by that.

    It also allows me to replace objects easily

    I will not argue that, but for QTextStream you can change the device in much the same way by using QTextStream::setDevice and QFile you can open and close multiple times if you need, by setting the file name and passing the open-mode flags.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.