Segmentation fault on App close

maximus

It's probably related to QWebView or QNetworkAccessManager, I use both and I'm thinking some QNetworkReply slot finished may point to a deleted object..

Seem to be related to the QWebView still trying to load the URL after I close the dialog.. will investigate and post result

Only happen on OSX atm..

maximus

After lot of debugging, I've isolated the problem:

When the QDialog is closed and the QWebView is still loading a webPage, the crashes happen, I haven't been able to manually stop loading of the QWebView, even using .stop(), still get the crash. Only happen on OSX not on windows

@ /// -------------- Dialog Login -----------------
bool LoginRefused = false;
DialogLogin *login = new DialogLogin();
//login.setModal(true);

if (login->exec&#40;&#41; != QDialog::Accepted)
    LoginRefused = true;

delete login;

if (LoginRefused) {

    /// delete objects
    delete managerWS;
    delete manager;
    delete version;
    delete settings;
    delete soundPlayer;
    delete account;

    qDebug() << ":not accepted dialog login";
    return 0; // Login refused
}
/// ---------------------------------------------@

@DialogLogin::~DialogLogin() {

qDebug() << "destructor DialogLogin";
ui->webView_login->stop();
delete ui

}
@

SGaist

Hi,

Two questions:

Why don't you allocate login on the stack ?

Why do you create all these object before the login phase ?

maximus

Hey SGaist,

It was originally on the stack, I tried with pointers to try to debug the problem, I went back to the stack approach now.

2- Theses objects are "global" variables used for the whole application. When I need the variable, I just cast it in the class.
Ex:
@this->account = qApp->property("Account").value<Account*>();@

Kind of like singleton, not the cleanest approach but it works. That way I can access important variables from any class.

The error seems to be related to QWebView getting deleted while it's loading a web page. I did a small workaround, not 100% bug-proof though, I show a loading icon while the QWebView is loading the page, then show the QWebView only when it's loaded. It's still possible to bug the app if you leave the QDialog when the QWebView is loading (on OSX only).

You can try it there if you want (test version for OS X) not yet released
https://maximumtrainer.com/MT.dmg

SGaist

Indeed, you should rather have a dedicated singleton for that kind of stuff.

Where's that web view used ? Do you need it for the login part ?

maximus

The QWebView is a private member of the QDialog class (custom QDialog used for login)
I need te QWebView for login, it's just loading a web page pointing to "https://maximumtrainer.com/login/insideMT":https://maximumtrainer.com/login/insideMT
Then I parse the result of the DialogLogin (parse json response of the QWebView) to open my MainWindow and create some object.

Not sure if singleton is better than the pointers? I don't think it can cause problem since all my GUI class that access theses pointer are in the same thread? I like the easy approach of global variables..

SGaist

Ok, so are you deleting the QWebView by hand rather than let the parent/child system of Qt do it for you ?

On a side note, why not use a web service for the login part ? That would avoid to load the webpage just for that purpose.

It's good practice to avoid global variables, you already have 6 of these. That might indicate a flaw in your application design. Easy approach like that generally means that your on your way to maintenance/debugging hell

maximus

I'm not deleting the QWebView by hand, just letting the the QDialog do it since it's the parent.

I could use a webService for login, i'm just reusing the web page I already have on my website, that way my app use the same code for login and it saves me some coding. I've decided to do all the interfaces of my app with QWebView for style reason (easier for cross-platform, easier and faster to develop than QWidgets, better looking..)

I'll think how to better redesign the global variables, thanks for your comments!

SGaist

Don't forget that if you go mobile, you won't have it on iOS nor Android

maximus

Thanks for the tip, I will consider changing it. No plan to support QWebView or the new QWebEngine for mobile? I hope all the code I did with QWebKit won't be too hard to port if I have to change.

On a different note, do you guys provide consulting ($). For example, looking at my whole project and giving me advice on what is wrong or bad coding/place subject to bugs. I'm having problem keeping up with the demand on my software. Thanks!

maximus

SGaist,

I was reading on how to replace my global variable to better code:
"http://qt-project.org/forums/viewthread/13549":http://qt-project.org/forums/viewthread/13549

Some people say Singleton should be avoided, i'm a bit confuse

I just need 1 object that will be accessed from multiple threads, the object is instantiated at the start of the program, what would you recommend?

Thanks!

maximus

After watching this "interesting video":https://www.youtube.com/watch?v=-FRm3VPhseI

I think I can get away with no singleton at all.

My problem is I like the use of singleton to access variable from anywhere, some place that is far from the object tree, I probably need to review my code to see how to access theses variable without global variable or singleton..

maximus

Sometimes, I think global variable or singleton can still be useful

Should I have to pass the QNetworkAccessManager in every function instead of using global variable?

@@QNetworkReply* VersionDAO::getVersion(QString lang, QString os) {

QNetworkAccessManager *managerWS = qApp->property("NetworkManagerWS").value<QNetworkAccessManager*>();


const QString url =  Environnement::getURLEnvironnementWS() + "api/version_rest/version" +
        "/lang/" + lang +
        "/os/"  + os +
        "/format/json";


qDebug() << url;

QNetworkRequest request;
request.setUrl(QUrl(url));
request.setRawHeader("User-Agent", "MyOwnBrowser 1.0");

QNetworkReply *replyLogin = managerWS->get(request);
qDebug() << "GET VERSION END";
return replyLogin;

}@@

SGaist

Singleton have their use.

In the case of your QNAM, are you using it only to make calls to your API ? If so you should rather have a specialized object dedicated to that rather that have your calls spread across your code. Take for example an update of your API path: how many files would you have to change ?

As for your consulting question, yes that can be done

maximus

Hey SGaist,

Exactly, I did put my API call only in API specific class. Then I can update only 1 functions and all the other using it are fine.

@class VersionDAO
{
public:

static QNetworkReply* getVersion(QString lang, QString os);

};@

I'm still using some global variable declared at the start of my main:

Account is an object that represent a table in my DB (account). I create this object on login, it can be modified/accessed from a few classes, then on close of the app, it is saved back to the DB using a webService.
Settings is all the settings my app have (using QSettings). I often need to check a specific settings before calling some code in most of my classes. Accessing it from everywhere is a great time saver.
SoundPlayer, I could probably instantiate this later only when I need it and move it out of the main. So this one could easily go
QNAM : I use one QNAM for all my webService call, and another for all QWebView inside my app, I had problem using the same for both so I had to use two.

@ /// Global variables
Account account = new Account();
app.setProperty("Account", QVariant::fromValue<Account>(account));

Settings *settings = new Settings();
app.setProperty("User_Settings", QVariant::fromValue<Settings*>(settings));

SoundPlayer *soundPlayer = new SoundPlayer(settings->soundPlayerVol);
app.setProperty("SoundPlayer", QVariant::fromValue<SoundPlayer*>(soundPlayer));


QNetworkAccessManager *manager = new QNetworkAccessManager();
app.setProperty("NetworkManager", QVariant::fromValue<QNetworkAccessManager*>(manager));
QNetworkAccessManager *managerWS = new QNetworkAccessManager();
app.setProperty("NetworkManagerWS", QVariant::fromValue<QNetworkAccessManager*>(managerWS));@

Let me know for consulting how it can be done. I could give access to the whole project and get a report of things to change/improve or something like this. I want the project to be easy to maintain and not debugging hell :)

Thanks in advance,
Max

[Edit: I could put the QNAM in the API classes, but I use 4 DAO classes, would putting a QNAM private member in each be good?, thanks for your help]

SGaist

You should send an email to start the contact, but yes access to the code will be mandatory. Are you using github, bitbucket, something else ?

maximus

Hi SGaist, I think i'll be fine for now, I learned a lot on Qt and starting to get the hang of it!
Here is my new stripped down main.

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

QApplication app(argc, argv);


/// Used in Signal/Slots connection
qRegisterMetaType<PowerCurve>("PowerCurve");
qRegisterMetaType<Sensor>("Sensor");
qRegisterMetaType<SensorEnum::SENSOR>("SensorEnum::SENSOR");
qRegisterMetaType<CalibrationType>("CalibrationType");


//initialize global object (Account, Settings, SoundPlayer and QNetworkAccessManager)
GlobalVars myVars;



/// App Stylesheet (hack so I can type stylesheet in designer instead of source code)
Z_StyleSheet styleSheetDummy;
app.setStyleSheet(styleSheetDummy.styleSheet());


DialogLogin login;
if (login.exec&#40;&#41; != QDialog::Accepted) {
    return 0; // Login refused
}
if (login.getGotUpdate()) {
    return 0; // Executed DialogLogin and redirected to download new version
}

/// Main program
MainWindow w;
w.show();

return app.exec();

}@

GlobalVar
@GlobalVars::GlobalVars(QObject *parent) :
QObject(parent)
{

///-------- INIT GLOBAL VAR ---------------------------
Account *account = new Account(this);
Settings *settings = new Settings(this);
SoundPlayer *soundPlayer = new SoundPlayer(settings->soundPlayerVol, this);

QNetworkAccessManager *manager = new QNetworkAccessManager(this);
QNetworkAccessManager *managerWS = new QNetworkAccessManager(this);


qApp->setProperty("Account", QVariant::fromValue<Account*>(account));
qApp->setProperty("User_Settings", QVariant::fromValue<Settings*>(settings));
qApp->setProperty("SoundPlayer", QVariant::fromValue<SoundPlayer*>(soundPlayer));
qApp->setProperty("NetworkManager", QVariant::fromValue<QNetworkAccessManager*>(manager));
qApp->setProperty("NetworkManagerWS", QVariant::fromValue<QNetworkAccessManager*>(managerWS));

}@

I make use of QObject parent to let do the memory management (had to inherit QObject on all my plain c++ objects).

Also too lazy to do get/set method and put the field private on theses objects, I know in java this is a must but in C++ people seem to be okay with that.
Thanks,
Max

maximus